Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing null check for cluster connections #100

Merged

Conversation

szehetner
Copy link
Contributor

This adds a null check in Dispose(ErrorHandler errorHandler, IDisposable disposable), similar to the other CloseHelper.Dispose methods.

Background:
When I try to connect to a cluster that is not running/responding, I do get the expected AeronTimeoutException from AeronCluster.Connect(). But additionally the ErrorHandler is called with a NullReferenceException.

The ErrorHandler is called from CloseHelper.Dispose:

at Adaptive.Agrona.CloseHelper.Dispose(ErrorHandler errorHandler, IDisposable disposable)
   at Adaptive.Cluster.Client.AeronCluster.AsyncConnect.Dispose()
   at Adaptive.Agrona.CloseHelper.QuietDispose(IDisposable disposable)
   at Adaptive.Cluster.Client.AeronCluster.Connect(Context ctx)

The NullReferenceException that is passed to the ErrorHandler has this StackTrace:
at Adaptive.Agrona.CloseHelper.Dispose(ErrorHandler errorHandler, IDisposable disposable)

The issue is that AsyncConnect.Dispose() calls CloseHelper.Dispose(errorHandler, ingressPublication) where ingressPublication might still be null. Additionally CloseHelper.Dispose(errorHandler, ingressPublication) is called twice in AsyncConnect.Dispose(), which looks unintentional, so I removed this too.

@JPWatson JPWatson merged commit d664df1 into AdaptiveConsulting:master Sep 28, 2020
@szehetner szehetner mentioned this pull request Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants