ReceiveFrameBytesAsync does not deregister from cancellationtoken, leading to a range of inner exceptions (also creates a memory leak)

abeham commented 3 years ago

Environment

NetMQ Version:    4.0.1.6
Operating System: Windows 10 20H2
.NET Version:     5.0

Expected behaviour

When terminating the service using Ctrl+C on the console I expect it to terminate without dozens or hundreds of unhandled exceptions.

Actual behaviour

The longer you let it run, the more inner exceptions are received, potentially thousands or more.

Receiving thousands of inner exceptions of the form: ---> (Inner Exception #34) System.InvalidOperationException: An attempt was made to transition a task to a final state when it had already completed. at System.Threading.Tasks.TaskCompletionSource1.SetCanceled(CancellationToken cancellationToken) at System.Threading.Tasks.TaskCompletionSource1.SetCanceled() at NetMQ.AsyncReceiveExtensions.<>cDisplayClass3_0.b0() at System.Threading.CancellationToken.<>c.<.cctor>b26_0(Object obj) at System.Threading.CancellationTokenSource.CallbackNode.<>c.b9_0(Object s) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location --- at System.Threading.CancellationTokenSource.CallbackNode.ExecuteCallback() at System.Threading.CancellationTokenSource.ExecuteCallbackHandlers(Boolean throwOnFirstException)<--- <---

Steps to reproduce the behaviour

I created an XPub/XSub proxy and a couple of services using a .NET 5.0 Worker Service project. The project is very simple and should show only the issue. The services only pass messages between each other following a delay of 100ms.

Clone this project: https://github.com/abeham/netmq-proxy
Open a terminal within this repository root folder
dotnet run -p Middleware
Hit Ctrl+C after some seconds

You experience a range of exceptions being printed to the console, the longer you wait before aborting with Ctrl+C, the more exceptions will be produced.

The problem

The method registeres a handler for the internally used TaskCompletionSource on the cancellation token. If that token is however injected from outside, the task completion source is still alive if the ReceiveFrameBytesAsync method terminates, because the source is never deregistered.

drewnoakes commented 3 years ago

This code...

https://github.com/zeromq/netmq/blob/0b98a648ef0dd7933b5819257dee97e64bfb6420/src/NetMQ/AsyncReceiveExtensions.cs#L87-L88

...should probably be using CreateLinkedToken instead of registering a callback.

It might also be sensible to use TrySetResult below, rather than SetResult.

Would you like to submit a pull request?

abeham commented 3 years ago

@jkarder will submit a pull request

zeromq / netmq