Skip to main content
Code Review

Return to Answer

broken link fixed, cf. https://meta.stackoverflow.com/a/406565/4751173
Source Link
svick
  • 24.5k
  • 4
  • 53
  • 89

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

broken link fixed, cf. https://meta.stackoverflow.com/a/406565/4751173
Source Link

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

Source Link
svick
  • 24.5k
  • 4
  • 53
  • 89

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

  1. The class is created in unset state.
  2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
  3. Set() is called, setting isSet to 1.
  4. The handler from step 2 is dequeued.
  5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
  6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
  7. The handler is Invoke()d, but TryReset() fails.
  8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.


I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /