-
Notifications
You must be signed in to change notification settings - Fork 15
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
Some patches #63
Some patches #63
Conversation
#endif | ||
} | ||
} | ||
catch (Exception ex) |
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.
Is it possible to narrow this exception type?
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.
Well the specific exception I was seeing was a SocketException, but is there really any value in letting any exception escape here?
… same key already exists in the dictionary."
@@ -205,7 +205,7 @@ protected virtual void OnConnectionMade (object sender, ConnectionMadeEventArgs | |||
return; | |||
|
|||
lock (this.connections) | |||
this.connections.Add (e.Connection, ExecutionMode.ConnectionOrder); | |||
this.connections [e.Connection] = ExecutionMode.ConnectionOrder; |
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? EDIT: I see your commit message for this. This really should never be an issue and seems like there's a larger bug at play that would be better to fix.
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.
Yeah, but I don't have any idea of what the larger bug could be. Any pointers? Here's the full trace, for the record:
System.ArgumentException: An element with the same key already exists in the dictionary.
at System.Collections.Generic.Dictionary`2[Tempest.IConnection,Tempest.ExecutionMode].Add (IConnection key, ExecutionMode value) [0x00000] in <filename unknown>:0
at Tempest.TempestServer.OnConnectionMade (System.Object sender, Tempest.ConnectionMadeEventArgs e) [0x00000] in <filename unknown>:0
at Tempest.Providers.Network.NetworkConnectionProvider.OnConnectionMade (Tempest.ConnectionMadeEventArgs e) [0x00000] in <filename unknown>:0
at Tempest.Providers.Network.NetworkConnectionProvider.Connect (Tempest.Providers.Network.NetworkServerConnection connection) [0x00000] in <filename unknown>:0
at Tempest.Providers.Network.NetworkServerConnection.OnTempestMessageReceived (Tempest.MessageEventArgs e) [0x00000] in <filename unknown>:0
at Tempest.Providers.Network.NetworkConnection.ReliableReceiveCompleted (System.Object sender, System.Net.Sockets.SocketAsyncEventArgs e) [0x00000] in <filename unknown>:0
at System.Net.Sockets.SocketAsyncEventArgs.OnCompleted (System.Net.Sockets.SocketAsyncEventArgs e) [0x00000] in <filename unknown>:0
at System.Net.Sockets.SocketAsyncEventArgs.ReceiveCallback (IAsyncResult ares) [0x00000] in <filename unknown>:0
at System.Net.Sockets.SocketAsyncEventArgs.DispatcherCB (IAsyncResult ares) [0x00000] in <filename unknown>:0
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.
Can you consistently reproduce this? If so, can you share the repro?
Here are a few patches I made while playing around. Hope they're helpful!