StephenCleary / AsyncEx

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

Expose IsEmpty from IAsyncWaitQueue on AsyncConditionVariable and AsyncMonitor #208

Open crazyjncsu opened 4 years ago

crazyjncsu commented 4 years ago

It would be useful to expose this state. We've had to write code like the following (with waiterCount) that could be avoided if this state was exposed through your API:

                try
                {
                    this.waiterCount++;

                    while (true)
                    {
                        await this.syncLock.WaitAsync(cancellationToken);

                        if (version < this.version)
                            return this.version;
                    }
                }
                finally
                {
                    this.waiterCount--;
                }

Propose that AsyncConditionVariable adds property:

public bool IsWaited => !this._queue.IsEmpty;

Propose that AsyncMonitor adds property:

public bool IsWaited => !this._conditionVariable.IsWaited;
StephenCleary commented 4 years ago

Why do you need waiterCount? What does this provide to your code?

crazyjncsu commented 4 years ago
            while (true)
            {
                using (await this.syncLock.EnterAsync())
                {
                    if (this.waiterCount == 0)
                    {
                        this.isWaitLoopRunning = false;
                        break;
                    }
                }

                var newVersion = // do stuff outside of lock

                using (await this.syncLock.EnterAsync())
                {
                    if (newVersion > this.version)
                    {
                        this.version = newVersion;
                        this.syncLock.PulseAll();
                    }
                }

                await Task.Delay(100);
            }

and in another place (right before the code I originally included):

            using (await this.syncLock.EnterAsync())
            {
                if (version < this.version)
                    return this.version;

                if (!this.isWaitLoopRunning)
                {
                    this.isWaitLoopRunning = true;
                    Task.Run(this.RunWaitLoop).CrashOnException();
                }

and there is another place in our code where we have a similar pattern, and the other developer also used a waiterCount increment/decrement to keep track of waiters.

StephenCleary commented 4 years ago

It looks like you're trying to create a kind of consumer/producer pattern with automatically-exiting and automatically-restarted consumers. In the async world, automatically exiting and automatically restarting is not usually necessary; it should be possible to structure this so that the consumer awaits its next "event" ("version" in your code), and then your consumer can start immediately and run indefinitely.

If that doesn't sound right, can you describe in words what you're actually trying to do?

The reason I'm asking is that AsyncEx is deliberately very specific about what state properties it exposes. Asking for additional state is usually an indication that the code is using the wrong synchronization primitive for what it is actually trying to do.