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

Fixed unhandled SemaphoreFullException when messages arrive while ReceiveAsync is still executing #74

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gribunin
Copy link

_readSemaphore has been replaced with _readEvent, because an event can be safely Set multiple times in a row, while semaphore can be unintentionally Release'ed beyond its maximum level. When server sends messages faster than the client reads, and OnChannelMessageReceived is called while ReceiveAsync is still executing, the _readSemaphore is Release'ed twice -- first in on OnChannelMessageReceived, then second time at the end of ReceiveAsync (because readItems.Count > 0) which lead to unhandled SemaphoreFullException in ReceiveAsync

The offending data flow:

  1. OnChannelMessageReceived (readSemaphore.Release()) ----- readSemaphore.Count == 1
  2. WaitOne is triggered at the beginning of ReceiveAsync ----- readSemaphore.Count == 0
  3. Another OnChannelMessageReceived (readSemaphore.Release()) --- readSemaphoreCount == 1
  4. End of ReceiveAsync (if (_readItems.Count > 0) readSemaphore.Release()) -- readSemaphoreCount == 2 -- SemaphoreFullException

…n be safely Set multiple times in a row, while semaphore can be Release'ed beyond its maximum level. When server sends messages faster than the client reads, and OnChannelMessageReceived was called while ReceiveAsync is still executing, the _readSemaphore was Release'ed twice -- first in on OnChannelMessageReceived, then second time at the end of ReceiveAsync (because readItems.Count > 0) which lead to unhandled SemaphoreFullException in ReceiveAsync
…ter was not used). More solid pattern handling of CancellationToken in ReceiveAsync
@fabianoriccardi
Copy link

Any news about this pull request? I had experienced the same problem while testing my webserver with jMeter.

I made a quick and dirty fix removing this line
But I don't know any possible consequences.

@Dragan-Juric
Copy link

Hello,
I have the same problem, is there any update on this?

@jgauffin
Copy link
Owner

I'm working on a new network stack. Completely async and rewritten from scratch.

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.

None yet

4 participants