/AzurePipelines run

@idg10, can we proceed with the merge? It seems the issue is still there.

Hi, sorry for how long this is taking.

I've got a couple of concerns:

• there are no tests around this bug
• I don't understand the thinking around why this is the fix
• is anything going to dispose the _ctr that was getting disposed back when it called Dispose instead of DisposeSubscription?
• presumably the call to _values.TryDequeue is still happening, it's just no longer failing because we're no longer nulling out the relevant fields, but is that OK? If feels like this change just masks a problem (which is that code is attempting to do things in this object after it should have been shut down)

Is there a reason for replacing the full Dispose with what seems to be a half-baked disposal instead of something like loading _values into a local variable and then checking it for null? Or trying to prevent the situation from occurring?

Hi @idg10,

It’s been a few years since I last looked at this, but I wanted to address some of your concerns:

• The issue is related to a race condition, which makes it challenging to create a reliable unit test without the risk of false positives. However, this functionality is already covered in the existing tests.
• Currently, on cancellation, ObservableAsyncEnumerable calls Dispose() on itself, which I found unconventional, especially since it handles some signaling logic afterward. This is problematic as the instance is still in use by the consumer (the enumerator pattern). Instead, we should unsubscribe and signal that it has completed.
• Dispose() should only be called by the consumer (the enumerator pattern) and not internally.
• The cancellation of the enumerable is controlled by _cancellationToken. If it is raised after the check on line 60, I believe we should allow the last item to be produced consistently before canceling the enumerable in the next cycle.
• Checking for null reliably without locking is tricky because three variables (_signal, _values, and _error) might be set or accessed by different threads.

Let me know if you have any further thoughts.