From 477fd1d90d29cb6a2cc43920b2bebc12e61d7e3b Mon Sep 17 00:00:00 2001
From: Pavel Krymets <pavel@krymets.com>
Date: Thu, 2 Aug 2018 13:17:17 -0700
Subject: [PATCH] Aquire exclusive lock when creating application info (#1142)

---
 .../NativeIISSample/NativeIISSample.csproj    |  1 +
 .../AspNetCore/applicationmanager.cpp         | 42 +++++++++++-----
 .../CommonStartupTests.cs                     | 48 +++++++++++++++++++
 3 files changed, 78 insertions(+), 13 deletions(-)
 create mode 100644 test/Common.FunctionalTests/CommonStartupTests.cs

diff --git a/samples/NativeIISSample/NativeIISSample.csproj b/samples/NativeIISSample/NativeIISSample.csproj
index d3c6d51b75..c1f4b88c89 100644
--- a/samples/NativeIISSample/NativeIISSample.csproj
+++ b/samples/NativeIISSample/NativeIISSample.csproj
@@ -4,6 +4,7 @@
 
   <PropertyGroup>
     <TargetFramework>netcoreapp2.2</TargetFramework>
+    <InProcessTestSite>true</InProcessTestSite>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp
index 9de2901be5..be4d124437 100644
--- a/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp
+++ b/src/AspNetCoreModuleV2/AspNetCore/applicationmanager.cpp
@@ -44,24 +44,40 @@ APPLICATION_MANAGER::GetOrCreateApplicationInfo(
     // key in the applicationInfoHash.
     pszApplicationId = pApplication.GetApplicationId();
 
-    // When accessing the m_pApplicationInfoHash, we need to acquire the application manager
-    // lock to avoid races on setting state.
-    SRWSharedLock lock(m_srwLock);
-    if (!m_fDebugInitialize)
     {
-        DebugInitializeFromConfig(m_pHttpServer, pApplication);
-        m_fDebugInitialize = TRUE;
-    }
+        // When accessing the m_pApplicationInfoHash, we need to acquire the application manager
+        // lock to avoid races on setting state.
+        SRWSharedLock readLock(m_srwLock);
+        if (!m_fDebugInitialize)
+        {
+            DebugInitializeFromConfig(m_pHttpServer, pApplication);
+            m_fDebugInitialize = TRUE;
+        }
 
-    if (g_fInShutdown)
-    {
-        FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS));
-    }
+        if (g_fInShutdown)
+        {
+            FINISHED(HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS));
+        }
 
-    m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo);
+        m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo);
+
+        // It's important to release read lock here so exclusive lock
+        // can be reacquired later as SRW lock doesn't allow upgrades
+    }
 
     if (*ppApplicationInfo == NULL)
     {
+        // Take exclusive lock before creating the application
+        SRWExclusiveLock writeLock(m_srwLock);
+
+        // Check if other thread created the application
+
+        m_pApplicationInfoHash->FindKey(pszApplicationId, ppApplicationInfo);
+        if (*ppApplicationInfo != NULL)
+        {
+            FINISHED(S_OK);
+        }
+
         pApplicationInfo = new APPLICATION_INFO(m_pHttpServer);
 
         FINISHED_IF_FAILED(pApplicationInfo->Initialize(pApplication));
@@ -187,7 +203,7 @@ APPLICATION_MANAGER::RecycleApplicationFromManager(
     DWORD                   dwPreviousCounter = 0;
     APPLICATION_INFO_HASH*  table = NULL;
     CONFIG_CHANGE_CONTEXT   context;
- 
+
     if (g_fInShutdown)
     {
         // We are already shutting down, ignore this event as a global configuration change event
diff --git a/test/Common.FunctionalTests/CommonStartupTests.cs b/test/Common.FunctionalTests/CommonStartupTests.cs
new file mode 100644
index 0000000000..760c1ad04b
--- /dev/null
+++ b/test/Common.FunctionalTests/CommonStartupTests.cs
@@ -0,0 +1,48 @@
+// 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.IO;
+using System.Linq;
+using System.Net;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Server.IIS.FunctionalTests.Utilities;
+using Microsoft.AspNetCore.Server.IntegrationTesting;
+using Microsoft.AspNetCore.Server.IntegrationTesting.IIS;
+using Microsoft.AspNetCore.Testing.xunit;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests
+{
+    [Collection(PublishedSitesCollection.Name)]
+    public class CommonStartupTests : IISFunctionalTestBase
+    {
+        private readonly PublishedSitesFixture _fixture;
+
+        public CommonStartupTests(PublishedSitesFixture fixture)
+        {
+            _fixture = fixture;
+        }
+
+        public static TestMatrix TestVariants
+            => TestMatrix.ForServers(DeployerSelector.ServerType)
+                .WithTfms(Tfm.NetCoreApp22)
+                .WithAllApplicationTypes()
+                .WithAllAncmVersions()
+                .WithAllHostingModels();
+
+        [ConditionalTheory]
+        [MemberData(nameof(TestVariants))]
+        public async Task StartupStress(TestVariant variant)
+        {
+            var deploymentParameters = _fixture.GetBaseDeploymentParameters(variant, publish: true);
+
+            var deploymentResult = await DeployAsync(deploymentParameters);
+
+            await Helpers.StressLoad(deploymentResult.HttpClient, "/HelloWorld", response => {
+                Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+                Assert.Equal("Hello World", response.Content.ReadAsStringAsync().GetAwaiter().GetResult());
+            });
+        }
+    }
+}