StephenCleary / AsyncEx

A helper library for async/await.
MIT License
3.53k stars 356 forks source link

A question regarding the AsyncProducerConsumerQueue #188

Open bboyle1234 opened 5 years ago

bboyle1234 commented 5 years ago

Not necessarily the best way to do this, but I just wanted to draw your attention @StephenCleary to this stackoverflow question as I thought it might interest you:

https://stackoverflow.com/questions/57606311/observable-wrapper-for-asyncproducerconsumerqueue

Thank you.

bboyle1234 commented 5 years ago

Another thought, the line token.ThrowIfCancelled() exists in that code because the AsyncProducerConsumerQueue.DequeueAsync(CancellationToken token) continues to dequeue items when the token is cancelled, if the item is available without any wait. In every single case in which I used the queue, (which is hundreds), this is undesired behaviour. (I need it to step reading the queue when cancelled even if there are items available). What was your reasoning for making it this way?

StefAnglr commented 5 years ago

Just wandered in here out of curiosity - I suppose one could compare this to AsyncLock.LockAsync(CancellationToken token), where calling with a cancelled token will still acquire the lock if it's available. In both cases, the token is only being used to cancel a wait, if a wait is necessary, akin to a hypothetical AsyncLock.TryLock or AsyncProducerConsumerQueue.TryDequeue method.

That said, I had assumed otherwise, and was just about to write some code based on that assumption. I'm glad I stumbled on this!

bboyle1234 commented 5 years ago

Lol i already wrote thousands of lines based on the assumption! (A testament to how awesome the async queue is)

Id like the inline documentation to be updated with this information so it can be seen in intellisense.

binki commented 5 years ago

What was your reasoning for making it this way?

I am not Stephen. I suspect it might have been modeled that way because that is like doing poll() with 0 for the timeout value—i.e., there is some precedent for this sort of style. It also provides a way to probe the queue in a way that is guaranteed(ish?) to run non-blocking synchronously and return immediately if there is nothing in the queue without having to add another parameter/overload dedicated to this. It also aligns with the concept that honoring CancellationToken is somewhat voluntary and that it is acceptable if the token is ignored when the request can be fulfilled synchronously (the called code never has to await, so it completed “instantly”).

Now, I also like to write code where I omit the cancellationToken.ThrowIfCancellationRequested() if I see that I am passing the token into some function because I assume that will result in an explosion if it is cancelled. While I kind of like the existing behavior because it is nifty, I agree that it is unexpected. However, I don’t think I have actually ever been bitten by this.

StephenCleary commented 4 years ago

Sorry I missed this!

The reasoning is summarized here: https://blog.stephencleary.com/2013/04/cancellationtoken-behavior-with-asyncex.html

It is a way to do "try" behavior, for one meaning of "try"; the multiple meanings of "try" are why I avoid that word in my names: https://github.com/StephenCleary/AsyncEx/blob/master/doc/design.md

I'll leave this open while considering the documentation request.