diff options
| -rw-r--r-- | src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs | 95 | ||||
| -rw-r--r-- | src/libraries/System.Net.Sockets/tests/FunctionalTests/DisposedSocketTests.cs | 1 |
2 files changed, 53 insertions, 43 deletions
diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs index 99380b31cb0..e3f57359e9e 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs @@ -184,14 +184,11 @@ namespace System.Net.Sockets { try { - Interop.Sys.SocketEvent* buffer = _buffer; - ConcurrentDictionary<IntPtr, SocketAsyncContextWrapper> handleToContextMap = _handleToContextMap; - ConcurrentQueue<SocketIOEvent> eventQueue = _eventQueue; - SocketAsyncContext? context = null; + SocketEventHandler handler = new SocketEventHandler(this); while (true) { int numEvents = EventBufferCount; - Interop.Error err = Interop.Sys.WaitForSocketEvents(_port, buffer, &numEvents); + Interop.Error err = Interop.Sys.WaitForSocketEvents(_port, handler.Buffer, &numEvents); if (err != Interop.Error.SUCCESS) { throw new InternalException(err); @@ -200,43 +197,7 @@ namespace System.Net.Sockets // The native shim is responsible for ensuring this condition. Debug.Assert(numEvents > 0, $"Unexpected numEvents: {numEvents}"); - bool enqueuedEvent = false; - foreach (var socketEvent in new ReadOnlySpan<Interop.Sys.SocketEvent>(buffer, numEvents)) - { - IntPtr handle = socketEvent.Data; - - if (handleToContextMap.TryGetValue(handle, out SocketAsyncContextWrapper contextWrapper) && (context = contextWrapper.Context) != null) - { - if (context.PreferInlineCompletions) - { - context.HandleEventsInline(socketEvent.Events); - } - else - { - Interop.Sys.SocketEvents events = context.HandleSyncEventsSpeculatively(socketEvent.Events); - - if (events != Interop.Sys.SocketEvents.None) - { - var ev = new SocketIOEvent(context, events); - eventQueue.Enqueue(ev); - enqueuedEvent = true; - - // This is necessary when the JIT generates unoptimized code (debug builds, live debugging, - // quick JIT, etc.) to ensure that the context does not remain referenced by this method, as - // such code may keep the stack location live for longer than necessary - ev = default; - } - } - - // This is necessary when the JIT generates unoptimized code (debug builds, live debugging, - // quick JIT, etc.) to ensure that the context does not remain referenced by this method, as - // such code may keep the stack location live for longer than necessary - context = null; - contextWrapper = default; - } - } - - if (enqueuedEvent) + if (handler.HandleSocketEvents(numEvents)) { ScheduleToProcessEvents(); } @@ -326,6 +287,56 @@ namespace System.Net.Sockets } } + // The JIT is allowed to arbitrarily extend the lifetime of locals, which may retain SocketAsyncContext references, + // indirectly preventing Socket instances to be finalized, despite being no longer referenced by user code. + // To avoid this, the event handling logic is delegated to a non-inlined processing method. + // See discussion: https://github.com/dotnet/runtime/issues/37064 + // SocketEventHandler holds an on-stack cache of SocketAsyncEngine members needed by the handler method. + private readonly struct SocketEventHandler + { + public Interop.Sys.SocketEvent* Buffer { get; } + + private readonly ConcurrentDictionary<IntPtr, SocketAsyncContextWrapper> _handleToContextMap; + private readonly ConcurrentQueue<SocketIOEvent> _eventQueue; + + public SocketEventHandler(SocketAsyncEngine engine) + { + Buffer = engine._buffer; + _handleToContextMap = engine._handleToContextMap; + _eventQueue = engine._eventQueue; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public bool HandleSocketEvents(int numEvents) + { + bool enqueuedEvent = false; + foreach (var socketEvent in new ReadOnlySpan<Interop.Sys.SocketEvent>(Buffer, numEvents)) + { + if (_handleToContextMap.TryGetValue(socketEvent.Data, out SocketAsyncContextWrapper contextWrapper)) + { + SocketAsyncContext context = contextWrapper.Context; + + if (context.PreferInlineCompletions) + { + context.HandleEventsInline(socketEvent.Events); + } + else + { + Interop.Sys.SocketEvents events = context.HandleSyncEventsSpeculatively(socketEvent.Events); + + if (events != Interop.Sys.SocketEvents.None) + { + _eventQueue.Enqueue(new SocketIOEvent(context, events)); + enqueuedEvent = true; + } + } + } + } + + return enqueuedEvent; + } + } + // struct wrapper is used in order to improve the performance of the epoll thread hot path by up to 3% of some TechEmpower benchmarks // the goal is to have a dedicated generic instantiation and using: // System.Collections.Concurrent.ConcurrentDictionary`2[System.IntPtr,System.Net.Sockets.SocketAsyncContextWrapper]::TryGetValueInternal(!0,int32,!1&) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/DisposedSocketTests.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/DisposedSocketTests.cs index ee81d7cdf81..e15249c19c3 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/DisposedSocketTests.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/DisposedSocketTests.cs @@ -751,7 +751,6 @@ namespace System.Net.Sockets.Tests [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] [InlineData(false)] [InlineData(true)] - [ActiveIssue("https://github.com/dotnet/runtime/issues/35846", TestPlatforms.AnyUnix)] public async Task NonDisposedSocket_SafeHandlesCollected(bool clientAsync) { List<WeakReference> handles = await CreateHandlesAsync(clientAsync); |