Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 8ae3df0

Browse files
authored
Fix BoundedChannel.DequeueItemAndPostProcess to respect _doneWriting (#26707)
It shouldn't attempt to interact with _blockedWriters/_waitingWriters if _doneWriting has been set.
1 parent 623af48 commit 8ae3df0

File tree

1 file changed

+33
-23
lines changed

1 file changed

+33
-23
lines changed

src/System.Threading.Channels/src/System/Threading/Channels/BoundedChannel.cs

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -117,30 +117,42 @@ private T DequeueItemAndPostProcess()
117117
// Dequeue an item.
118118
T item = parent._items.DequeueHead();
119119

120-
// If we're now empty and we're done writing, complete the channel.
121-
if (parent._doneWriting != null && parent._items.IsEmpty)
120+
if (parent._doneWriting != null)
122121
{
123-
ChannelUtilities.Complete(parent._completion, parent._doneWriting);
122+
// We're done writing, so if we're now empty, complete the channel.
123+
if (parent._items.IsEmpty)
124+
{
125+
ChannelUtilities.Complete(parent._completion, parent._doneWriting);
126+
}
124127
}
125-
126-
// If there are any writers blocked, there's now room for at least one
127-
// to be promoted to have its item moved into the items queue. We need
128-
// to loop while trying to complete the writer in order to find one that
129-
// hasn't yet been canceled (canceled writers transition to canceled but
130-
// remain in the physical queue).
131-
while (!parent._blockedWriters.IsEmpty)
128+
else
132129
{
133-
WriterInteractor<T> w = parent._blockedWriters.DequeueHead();
134-
if (w.Success(default))
130+
// If there are any writers blocked, there's now room for at least one
131+
// to be promoted to have its item moved into the items queue. We need
132+
// to loop while trying to complete the writer in order to find one that
133+
// hasn't yet been canceled (canceled writers transition to canceled but
134+
// remain in the physical queue).
135+
//
136+
// (It's possible for _doneWriting to be non-null due to Complete
137+
// having been called but for there to still be blocked/waiting writers.
138+
// This is a temporary condition, after which Complete has set _doneWriting
139+
// and then exited the lock; at that point it'll proceed to clean this up,
140+
// so we just ignore them.)
141+
142+
while (!parent._blockedWriters.IsEmpty)
135143
{
136-
parent._items.EnqueueTail(w.Item);
137-
return item;
144+
WriterInteractor<T> w = parent._blockedWriters.DequeueHead();
145+
if (w.Success(default))
146+
{
147+
parent._items.EnqueueTail(w.Item);
148+
return item;
149+
}
138150
}
139-
}
140151

141-
// There was no blocked writer, so see if there's a WaitToWriteAsync
142-
// we should wake up.
143-
ChannelUtilities.WakeUpWaiters(ref parent._waitingWriters, result: true);
152+
// There was no blocked writer, so see if there's a WaitToWriteAsync
153+
// we should wake up.
154+
ChannelUtilities.WakeUpWaiters(ref parent._waitingWriters, result: true);
155+
}
144156

145157
// Return the item
146158
return item;
@@ -187,11 +199,9 @@ public override bool TryComplete(Exception error)
187199
// will be the one to transition from _doneWriting false to true. As such, we can
188200
// freely manipulate them without any concurrency concerns.
189201

190-
// The following 3 line are not safe to do it outside the lock https://github.com/dotnet/corefx/issues/26587
191-
ChannelUtilities.FailInteractors<WriterInteractor<T>, VoidResult>(parent._blockedWriters, ChannelUtilities.CreateInvalidCompletionException(error));
192-
ChannelUtilities.WakeUpWaiters(ref parent._waitingReaders, result: false, error: error);
193-
ChannelUtilities.WakeUpWaiters(ref parent._waitingWriters, result: false, error: error);
194-
202+
ChannelUtilities.FailInteractors<WriterInteractor<T>, VoidResult>(parent._blockedWriters, ChannelUtilities.CreateInvalidCompletionException(error));
203+
ChannelUtilities.WakeUpWaiters(ref parent._waitingReaders, result: false, error: error);
204+
ChannelUtilities.WakeUpWaiters(ref parent._waitingWriters, result: false, error: error);
195205

196206
// Successfully transitioned to completed.
197207
return true;

0 commit comments

Comments
 (0)