SocketAsyncContext should not need to pin Span buffers for sync APIs

[geoffkizer]

Currently, if you call a sync Span-based Socket overload like Socket.Receive(Span<byte> buffer), and we end up going through the sync-over-async path in SocketAsyncContext, we will end up pinning the buffer when data is not immediately available from the socket.

See references to BufferPtrReceiveOperation and BufferPtrSendOperation in SocketAsyncContext.

In theory, this shouldn't be necessary. All sync operation handling happens on the original calling thread, so we can simply hold on to the Span there. The problem is that the structure of the code currently requires that we store this state on the Operation object, which means we need to pin the Span and treat it as a native pointer.

We should fix this. This would also allow us to remove the duplication of code that currently exists between the byte[] sync overloads and the Span sync overloads.

More generally, we shouldn't really need to store any operation-specific state on the Operation objects for sync operations; it can all live on the calling stack. This would likely simplify the code, and make these operations more efficient since we don't need to allocate an operation each time.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently, if you call a sync Span-based Socket overload like Socket.Receive(Span<byte> buffer), and we end up going through the sync-over-async path in SocketAsyncContext, we will end up pinning the buffer when data is not immediately available from the socket.

See references to BufferPtrReceiveOperation and BufferPtrSendOperation in SocketAsyncContext.

In theory, this shouldn't be necessary. All sync operation handling happens on the original calling thread, so we can simply hold on to the Span there. The problem is that the structure of the code currently requires that we store this state on the Operation object, which means we need to pin the Span and treat it as a native pointer.

We should fix this. This would also allow us to remove the duplication of code that currently exists between the byte[] sync overloads and the Span sync overloads.

More generally, we shouldn't really need to store any operation-specific state on the Operation objects for sync operations; it can all live on the calling stack. This would likely simplify the code, and make these operations more efficient since we don't need to allocate an operation each time.

Author:	geoffkizer
Assignees:	-
Labels:	area-System.Net.Sockets, tenet-performance
Milestone:	Future

[antonfirsov]

We should also find new patterns of code sharing to consolidate sync method duplications in Socket.cs.

For example Receive(byte[] ...), Receive(Span<byte> ...) and Receive(IList<ArraySegment<byte>> ...) are mostly duplicates, and the ReceiveMessageFrom duplication introduced in #46285 is even worse.