Request.CallCancelled is not set when client disconnects too fast

[maxcherednik]

Request.CallCancelled cancellation token is never signaled if a client disconnects before this cancellation observed.

Bellow you can find an example which imitates client's disconnection.
Case 1: we observe(access it) the Request.CallCancelled before client disconnects

Case 2: we observe(access it) the Request.CallCancelled when client already gone

using System;
using Owin;
using Microsoft.Owin.Hosting;
using System.Threading.Tasks;
using System.Threading;
using System.Net.Http;

namespace OwinCallCancelledRace
{
    public class Program
    {
        public static void Main(string[] args)
        {
            MainAsync().Wait();

            Console.ReadKey();
        }

        private static async Task MainAsync()
        {
            var url = "http://localhost:9000";

            var serverFinished = new TaskCompletionSource<bool>();

            var clientDisposed = new TaskCompletionSource<bool>();

            var failed = false;

            using (var clientRequestCancellationTokenSource = new CancellationTokenSource())
            using (var webApp = WebApp.Start(url, appBuilder => appBuilder.Run(async env =>
            {
                try
                {
                    Console.WriteLine("[Server] Request arrived");

                    // if we observe a cancellationToken while client is still connected, it works
                    //var ct = env.Request.CallCancelled;

                    clientRequestCancellationTokenSource.Cancel();

                    await clientDisposed.Task.ConfigureAwait(false);

                    // if we observe a cancellationToken when client already gone, the cancel never set
                    var ct = env.Request.CallCancelled;

                    // imitate long running job
                    await Task.Delay(5000, ct).ConfigureAwait(false);

                    failed = true;
                }
                catch (OperationCanceledException)
                {
                    Console.WriteLine("[Server] Request cancelled");
                }

                serverFinished.SetResult(true);
            })))
            {

                using (var client = new HttpClient())
                {
                    try
                    {
                        var response = await client.GetAsync(url, clientRequestCancellationTokenSource.Token);
                    }
                    catch (OperationCanceledException)
                    {
                        Console.WriteLine("[Client] Request cancelled");
                    }
                }

                clientDisposed.SetResult(true);

                await serverFinished.Task;

                Console.WriteLine("Server failed to observe cancellation: " + failed);
            }
        }
    }    
}

It seems the problem is somewhere here:

private unsafe CancellationToken CreateToken(ulong connectionId)
        {
            // Create a nativeOverlapped callback so we can register for disconnect callback
            var overlapped = new Overlapped();
            var cts = new CancellationTokenSource();
            CancellationToken returnToken = cts.Token;

            NativeOverlapped* nativeOverlapped = overlapped.UnsafePack(
                (errorCode, numBytes, overlappedPtr) =>
                {
                    // Free the overlapped
                    Overlapped.Free(overlappedPtr);

                    if (errorCode != NativeMethods.HttpErrors.NO_ERROR)
                    {
                        LogHelper.LogException(_logger, "IOCompletionCallback", new Win32Exception((int)errorCode));
                    }

                    // Pull the token out of the list and Cancel it.
                    ConnectionCancellation cancellation;
                    _connectionCancellationTokens.TryRemove(connectionId, out cancellation);

                    bool success = ThreadPool.UnsafeQueueUserWorkItem(CancelToken, cts);
                    Debug.Assert(success, "Unable to queue disconnect notification.");
                },
                null);

            uint hr = NativeMethods.HttpWaitForDisconnect(_requestQueueHandle, connectionId, nativeOverlapped);

            if (hr != NativeMethods.HttpErrors.ERROR_IO_PENDING &&
                hr != NativeMethods.HttpErrors.NO_ERROR)
            {
                // We got an unknown result, assume the connection has been closed.
                Overlapped.Free(nativeOverlapped);
                ConnectionCancellation cancellation;
                _connectionCancellationTokens.TryRemove(connectionId, out cancellation);
                LogHelper.LogException(_logger, "HttpWaitForDisconnectEx", new Win32Exception((int)hr));
                cts.Cancel();
            }

            return returnToken;
        }

Client already gone by the time we are subscribing for the disconnection event. Subscription itself doesn't not return any error code, but the callback never called.

[maxcherednik]

Seems that this part is missing:
https://github.com/aspnet/HttpSysServer/blob/231cbffe1a5cc0abce11e2d0685683ed74fc7ad5/src/Microsoft.AspNetCore.Server.HttpSys/NativeInterop/DisconnectListener.cs#L115

[Tratcher]

Interesting. That behavior is OS specific (Win8+), what OS are you running? See http://referencesource.microsoft.com/#System/net/System/Net/HttpListener.cs,296

[maxcherednik]

Hi @Tratcher, thank you for replying.

Windows 10. And the response is exactly NativeMethods.HttpErrors.NO_ERROR.
I can post a pull request with the test highlighting the issue.
I am just thinking that this documentation is not clear enough(regardsless the windows version) https://msdn.microsoft.com/en-us/library/windows/desktop/aa364508(v=vs.85).aspx

It is saying that:

Return value

If the function succeeds, the return value is NO_ERROR.
If the function is used asynchronously, a return value of ERROR_IO_PENDING indicates that the next request is not yet ready and is retrieved later through normal overlapped I/O completion mechanisms.
If the function fails, the return value is one of the following error codes.

So we do call it in async way(we provide the callback) and they say it should always return ERROR_IO_PENDING. Which means a callback will be called. but if we receive NO_ERROR - it means the overlapped operation is done, hence callback will not be called.

Btw, I know the case is very rare(client disconnection before we subscribe for the callback), even though it's a subject for a memory leak - since token will never be removed from the dictionary.

[maxcherednik]

And another thing regarding the code you sent. This code sample is on asp.net core. In katana this condition check is missing.

[maxcherednik]

A quote from here: Synchronization and Overlapped Input and Output

When a function is called to perform an overlapped operation, the operation might be completed before the function returns. When this happens, the results are handled as if the operation had been performed synchronously. If the operation was not completed, however, the function's return value is FALSE, and the GetLastError function returns ERROR_IO_PENDING.

So in our case we do receive NO_ERROR, which means async operation we were asking for finished in a synchronous way.