@geoffkizer commented in #37974 (comment):
@tmds I happened to be looking at this code and I'm confused by something.
You added the bool processAsyncEvents = true argument to ProcessSyncEventOrGetAsyncEvent, but it doesn't seem like this is actually getting set to false anywhere. All callers are either explicitly passing true or using the default arg value, which is true.
This should have been be set to false when it gets called in HandleEvents to avoid processing an AsyncOperation multiple times.
|
// Called on ThreadPool thread. |
|
public unsafe void HandleEvents(Interop.Sys.SocketEvents events) |
|
{ |
|
Debug.Assert((events & Interop.Sys.SocketEvents.Error) == 0); |
|
|
|
AsyncOperation? receiveOperation = |
|
(events & Interop.Sys.SocketEvents.Read) != 0 ? _receiveQueue.ProcessSyncEventOrGetAsyncEvent(this) : null; |
|
AsyncOperation? sendOperation = |
|
(events & Interop.Sys.SocketEvents.Write) != 0 ? _sendQueue.ProcessSyncEventOrGetAsyncEvent(this) : null; |
|
|
|
// This method is called from a thread pool thread. When we have only one operation to process, process it |
|
// synchronously to avoid an extra thread pool work item. When we have two operations to process, processing both |
|
// synchronously may delay the second operation, so schedule one onto the thread pool and process the other |
|
// synchronously. There might be better ways of doing this. |
|
if (sendOperation == null) |
|
{ |
|
receiveOperation?.Process(); |
|
} |
|
else |
|
{ |
|
receiveOperation?.Schedule(); |
|
sendOperation.Process(); |
|
} |
|
} |
Calling Process multiple times can lead to weird issues. When the first call leads to completing the operation, the instance may be reused for another operation by the time Process gets called the second time.
cc @karelz @antonfirsov @stephentoub @dotnet/ncl