Revert "Disconnect the disconnect handler when request processing ends (#1413)"

This reverts commit f808bdc331.

src/AspNetCoreModuleV2/AspNetCore/DisconnectHandler.cpp

@@ -4,21 +4,15 @@
 #include "DisconnectHandler.h"
 #include "exceptions.h"
 #include "proxymodule.h"
-#include "SRWExclusiveLock.h"
 
 void DisconnectHandler::NotifyDisconnect()
 {
     try
     {
-        std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER> module;
+        const auto module = m_pModule.exchange(nullptr);
-        {
-            SRWExclusiveLock lock(m_handlerLock);
-            m_pHandler.swap(module);
-        }
-
         if (module != nullptr)
         {
-            module->NotifyDisconnect();
+            module ->NotifyDisconnect();
         }
     }
     catch (...)
@@ -33,8 +27,7 @@ void DisconnectHandler::CleanupStoredContext() noexcept
     delete this;
 }
 
-void DisconnectHandler::SetHandler(std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER> handler) noexcept
+void DisconnectHandler::SetHandler(ASPNET_CORE_PROXY_MODULE * module) noexcept
 {
-    SRWExclusiveLock lock(m_handlerLock);
+    m_pModule = module;
-    handler.swap(m_pHandler);
 }

src/AspNetCoreModuleV2/AspNetCore/DisconnectHandler.h

@@ -2,9 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 #pragma once
-
+#include <atomic>
-#include <memory>
-#include "irequesthandler.h"
 
 class ASPNET_CORE_PROXY_MODULE;
 
@@ -12,9 +10,8 @@ class DisconnectHandler final: public IHttpConnectionStoredContext
 {
 public:
     DisconnectHandler()
-        : m_pHandler(nullptr)
+        : m_pModule(nullptr)
     {
-        InitializeSRWLock(&m_handlerLock);
     }
 
     virtual
@@ -30,10 +27,9 @@ public:
     CleanupStoredContext() noexcept override;
 
     void
-    SetHandler(std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER> handler) noexcept;
+    SetHandler(ASPNET_CORE_PROXY_MODULE * module) noexcept;
 
 private:
-    SRWLOCK m_handlerLock {};
+    std::atomic<ASPNET_CORE_PROXY_MODULE*> m_pModule;
-    std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER> m_pHandler;
 };
 

src/AspNetCoreModuleV2/AspNetCore/proxymodule.cpp

@@ -71,10 +71,6 @@ ASPNET_CORE_PROXY_MODULE::ASPNET_CORE_PROXY_MODULE(HTTP_MODULE_ID moduleId, std:
 
 ASPNET_CORE_PROXY_MODULE::~ASPNET_CORE_PROXY_MODULE()
 {
-    // At this point m_pDisconnectHandler should be disconnected in
-    // HandleNotificationStatus
-    assert(m_pDisconnectHandler == nullptr);
-
     if (m_pDisconnectHandler != nullptr)
     {
         m_pDisconnectHandler->SetHandler(nullptr);
@@ -116,6 +112,8 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler(
             FINISHED_IF_FAILED(moduleContainer->SetConnectionModuleContext(static_cast<IHttpConnectionStoredContext*>(disconnectHandler.release()), m_moduleId));
         }
 
+        m_pDisconnectHandler->SetHandler(this);
+
         FINISHED_IF_FAILED(m_pApplicationManager->GetOrCreateApplicationInfo(
             *pHttpContext,
             m_pApplicationInfo));
@@ -123,8 +121,6 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler(
         FINISHED_IF_FAILED(m_pApplicationInfo->CreateHandler(*pHttpContext, m_pHandler));
 
         retVal = m_pHandler->OnExecuteRequestHandler();
-
-        m_pDisconnectHandler->SetHandler(::ReferenceRequestHandler(m_pHandler.get()));
     }
     catch (...)
     {
@@ -145,7 +141,7 @@ Finished:
         }
     }
 
-    return HandleNotificationStatus(retVal);
+    return retVal;
 }
 
 __override
@@ -160,27 +156,18 @@ ASPNET_CORE_PROXY_MODULE::OnAsyncCompletion(
 {
     try
     {
-        return HandleNotificationStatus(m_pHandler->OnAsyncCompletion(
+        return m_pHandler->OnAsyncCompletion(
             pCompletionInfo->GetCompletionBytes(),
-            pCompletionInfo->GetCompletionStatus()));
+            pCompletionInfo->GetCompletionStatus());
     }
     catch (...)
     {
         OBSERVE_CAUGHT_EXCEPTION();
-        return HandleNotificationStatus(RQ_NOTIFICATION_FINISH_REQUEST);
+        return RQ_NOTIFICATION_FINISH_REQUEST;
     }
 }
 
-REQUEST_NOTIFICATION_STATUS ASPNET_CORE_PROXY_MODULE::HandleNotificationStatus(REQUEST_NOTIFICATION_STATUS status) noexcept
+void ASPNET_CORE_PROXY_MODULE::NotifyDisconnect() const
 {
-    if (status != RQ_NOTIFICATION_PENDING)
+    m_pHandler->NotifyDisconnect();
-    {
-        if (m_pDisconnectHandler != nullptr)
-        {
-            m_pDisconnectHandler->SetHandler(nullptr);
-            m_pDisconnectHandler = nullptr;
-        }
-    }
-
-    return status;
 }

src/AspNetCoreModuleV2/AspNetCore/proxymodule.h

@@ -47,8 +47,8 @@ class ASPNET_CORE_PROXY_MODULE : NonCopyable, public CHttpModule
         IHttpCompletionInfo *   pCompletionInfo
     ) override;
 
-    REQUEST_NOTIFICATION_STATUS
+    void
-    HandleNotificationStatus(REQUEST_NOTIFICATION_STATUS status) noexcept;
+    NotifyDisconnect() const;
 
  private:
     std::shared_ptr<APPLICATION_MANAGER> m_pApplicationManager;

src/AspNetCoreModuleV2/CommonLib/SRWExclusiveLock.cpp

@@ -3,7 +3,7 @@
 
 #include "SRWExclusiveLock.h"
 
-SRWExclusiveLock::SRWExclusiveLock(const SRWLOCK& lock) noexcept
+SRWExclusiveLock::SRWExclusiveLock(const SRWLOCK& lock)
     : m_lock(lock)
 {
     AcquireSRWLockExclusive(const_cast<SRWLOCK*>(&m_lock));

src/AspNetCoreModuleV2/CommonLib/SRWExclusiveLock.h

@@ -8,7 +8,7 @@
 class SRWExclusiveLock
 {
 public:
-	SRWExclusiveLock(const SRWLOCK& lock) noexcept;
+	SRWExclusiveLock(const SRWLOCK& lock);
 	~SRWExclusiveLock();
 private:
     const SRWLOCK& m_lock;

src/AspNetCoreModuleV2/CommonLib/irequesthandler.h

@@ -54,9 +54,3 @@ struct IREQUEST_HANDLER_DELETER
         application->DereferenceRequestHandler();
     }
 };
-
-inline std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER> ReferenceRequestHandler(IREQUEST_HANDLER* handler)
-{
-    handler->ReferenceRequestHandler();
-    return std::unique_ptr<IREQUEST_HANDLER, IREQUEST_HANDLER_DELETER>(handler);
-};

test/Common.FunctionalTests/ClientDisconnectStress.cs

@@ -1,65 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.Collections.Generic;
-using System.Net.Http;
-using System.Threading.Tasks;
-using Microsoft.AspNetCore.Server.IntegrationTesting;
-using Microsoft.AspNetCore.Testing.xunit;
-using Xunit;
-
-namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests
-{
-    [Collection(PublishedSitesCollection.Name)]
-    public class ClientDisconnectStressTests: FunctionalTestsBase
-    {
-        private readonly PublishedSitesFixture _fixture;
-
-        public ClientDisconnectStressTests(PublishedSitesFixture fixture)
-        {
-            _fixture = fixture;
-        }
-
-        [ConditionalTheory]
-        [InlineData(HostingModel.InProcess)]
-        [InlineData(HostingModel.OutOfProcess)]
-        public async Task ClientDisconnectStress(HostingModel hostingModel)
-        {
-            var site = await StartAsync(_fixture.GetBaseDeploymentParameters(hostingModel));
-            var maxRequestSize = 1000;
-            var blockSize = 40;
-            var random = new Random();
-            async Task RunRequests()
-            {
-                using (var connection = new TestConnection(site.HttpClient.BaseAddress.Port))
-                {
-                    await connection.Send(
-                        "POST /ReadAndFlushEcho HTTP/1.1",
-                        $"Content-Length: {maxRequestSize}",
-                        "Host: localhost",
-                        "Connection: close",
-                        "",
-                        "");
-
-                    var disconnectAfter = random.Next(maxRequestSize);
-                    var data = new byte[blockSize];
-                    for (int i = 0; i < disconnectAfter / blockSize; i++)
-                    {
-                        await connection.Stream.WriteAsync(data);
-                    }
-                }
-            }
-
-            List<Task> tasks = new List<Task>();
-            for (int i = 0; i < 100; i++)
-            {
-                tasks.Add(Task.Run(RunRequests));
-            }
-
-            await Task.WhenAll(tasks);
-
-            StopServer();
-        }
-    }
-}

test/WebSites/InProcessWebSite/Startup.cs

@@ -324,17 +324,6 @@ namespace TestSite
                 result = await ctx.Request.Body.ReadAsync(readBuffer, 0, readBuffer.Length);
             }
         }
-        private async Task ReadAndFlushEcho(HttpContext ctx)
-        {
-            var readBuffer = new byte[4096];
-            var result = await ctx.Request.Body.ReadAsync(readBuffer, 0, readBuffer.Length);
-            while (result != 0)
-            {
-                await ctx.Response.WriteAsync(Encoding.UTF8.GetString(readBuffer, 0, result));
-                await ctx.Response.Body.FlushAsync();
-                result = await ctx.Request.Body.ReadAsync(readBuffer, 0, readBuffer.Length);
-            }
-        }
 
         private async Task ReadAndWriteEchoLines(HttpContext ctx)
         {