-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
#2008 Follow-ups #2009
#2008 Follow-ups #2009
Conversation
A few tweaks to the changes #2008 for disposing and normalization.
@@ -113,6 +113,7 @@ public enum State : byte | |||
public void Dispose() | |||
{ | |||
isDisposed = true; | |||
_backlogAutoReset?.Dispose(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ?.
- this is readonly
, don't see how it could end up null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always like to guard in case this implementation changes later - agreed it's not needed atm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose in the absence of nullable reference types that's reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aye - this project isn't too large...should take that on one of these days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the other awaits I saw in ConnectionMultiplexer.cs without ForAwait()
:
await srv.PingAsync(flags); // if it isn't happy, we're not happy
await node.WriteDirectAsync(msg, ResultProcessor.DemandOK);
await srv.ReplicaOfAsync(null, flags);
await server.WriteDirectAsync(msg, ResultProcessor.DemandOK);
await node.WriteDirectAsync(msg, ResultProcessor.Int64);
await BroadcastAsync(nodes);
await node.WriteDirectAsync(msg, ResultProcessor.DemandOK);
await BroadcastAsync(nodes);
if (!await ReconfigureAsync(first: false, reconfigureAll: true, log, srv.EndPoint, "make master"))
return await ReconfigureAsync(first: false, reconfigureAll: true, logProxy, null, "configure").ObserveErrors();
@philon-msft ACK - for completeness let's handle those separately but agreed no reason not to tidy as long as we're supporting full framework. |
A few tweaks to the changes #2008 for disposing and normalization.