jamiewest / signalr_core

ASP.NET Core SignalR Dart Client
https://pub.dev/packages/signalr_core
MIT License
90 stars 63 forks source link

Race Condition with Multiple Stream Methods #3

Closed paulquinn closed 4 years ago

paulquinn commented 4 years ago

I can successfully connect to my server hubs with streaming methods using C# and Typescript clients. However, using this package I get an exception.

Setup

I have two subsequent streaming methods declared:

 var messageStream1 = this.connection.stream("serverStreamingMethod1", args: []);
 var messageStream2 = this.connection.stream("serverStreamingMethod2", args: []);

Outcome

Diagnosis

I've dug/stepped through the code and the error seems to be in http_connection.dart. The Future the error message (Bad state: Future already completed) is complaining about is the _sendBufferedData one returned at the end of the void _bufferData(dynamic data) method (this can be verified by adding a print(_sendBufferedData.isCompleted) statement when using multiple streaming methods. I'm not sure why this is the case (comparing to the TS client code) - but it looks like the buffer is being sent (and that operation not being completed) while other data is being added to the buffer.

Fix

The fix is easy... adding a guard around the _sendBufferedData.complete(); call in the _bufferData method fixes the issue, ie:

if(!_sendBufferedData.isCompleted) {
      _sendBufferedData.complete();
}

...this makes general sense as it means that the _buffer actually buffers data until the last buffered contents have been send successfully in the send loop. I'm not sure why that guard isn't in the TS client, though it seems to work without it...

I've tested this via Flutter Web and Android and it seems to work.

If you're open to a pull request, I can do that.

jamiewest commented 4 years ago

Thank you for your detailed issue, a PR would be greatly appreciated!