diff --git a/build/native.targets b/build/native.targets
new file mode 100644
index 0000000000..1c5a981691
--- /dev/null
+++ b/build/native.targets
@@ -0,0 +1,25 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/build/repo.targets b/build/repo.targets
index 3c932491ad..161887405e 100644
--- a/build/repo.targets
+++ b/build/repo.targets
@@ -7,9 +7,9 @@
$(RepositoryRoot)NuGetPackageVerifier.xplat.json
-
+
- -p:Configuration=$(Configuration) -v:m -nologo -clp:NoSummary
+ -p:Configuration=$(Configuration) -v:m -nologo -clp:NoSummary -p:CommitHash=$(CommitHash)
diff --git a/src/AspNetCore/AspNetCore.vcxproj b/src/AspNetCore/AspNetCore.vcxproj
index 675ae4cb87..4eee0392c1 100644
--- a/src/AspNetCore/AspNetCore.vcxproj
+++ b/src/AspNetCore/AspNetCore.vcxproj
@@ -227,24 +227,6 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
{55494e58-e061-4c4c-a0a8-837008e72f85}
@@ -272,6 +254,7 @@
+
diff --git a/src/AspNetCore/aspnetcoremodule.rc b/src/AspNetCore/aspnetcoremodule.rc
index a35cbf0475..657dfe3712 100644
--- a/src/AspNetCore/aspnetcoremodule.rc
+++ b/src/AspNetCore/aspnetcoremodule.rc
@@ -10,6 +10,8 @@
#if !defined(AFX_RESOURCE_DLL) || defined(AFX_TARG_ENU)
LANGUAGE LANG_ENGLISH, SUBLANG_ENGLISH_US
+#define FileDescription "IIS AspNetCore Module. Commit: " CommitHash
+
/////////////////////////////////////////////////////////////////////////////
//
// 11
@@ -75,7 +77,7 @@ BEGIN
BLOCK "040904b0"
BEGIN
VALUE "CompanyName", "Microsoft Corporation"
- VALUE "FileDescription", "IIS AspNetCore Module"
+ VALUE "FileDescription", FileDescription
VALUE "FileVersion", FileVersionStr
VALUE "InternalName", "aspnetcore"
VALUE "LegalCopyright", "Copyright (C) Microsoft Corporation"
diff --git a/src/CommonLib/resources.h b/src/CommonLib/resources.h
index 31b981d0fa..adba68b794 100644
--- a/src/CommonLib/resources.h
+++ b/src/CommonLib/resources.h
@@ -21,6 +21,7 @@
#define ASPNETCORE_EVENT_INVALID_STDOUT_LOG_FILE_MSG L"Warning: Could not create stdoutLogFile %s, ErrorCode = '0x%x'."
#define ASPNETCORE_EVENT_GRACEFUL_SHUTDOWN_FAILURE_MSG L"Failed to gracefully shutdown process '%d'."
#define ASPNETCORE_EVENT_SENT_SHUTDOWN_HTTP_REQUEST_MSG L"Sent shutdown HTTP message to process '%d' and received http status '%d'."
+#define ASPNETCORE_EVENT_APP_SHUTDOWN_FAILURE_MSG L"Failed to gracefully shutdown application '%s'."
#define ASPNETCORE_EVENT_LOAD_CLR_FALIURE_MSG L"Application '%s' with physical root '%s' failed to load clr and managed application, ErrorCode = '0x%x."
#define ASPNETCORE_EVENT_DUPLICATED_INPROCESS_APP_MSG L"Only one inprocess application is allowed per IIS application pool. Please assign the application '%s' to a different IIS application pool."
#define ASPNETCORE_EVENT_MIXED_HOSTING_MODEL_ERROR_MSG L"Mixed hosting model is not supported. Application '%s' configured with different hostingModel value '%d' other than the one of running application(s)."
diff --git a/src/RequestHandler/RequestHandler.vcxproj b/src/RequestHandler/RequestHandler.vcxproj
index a94bfb248d..fd198ba5b3 100644
--- a/src/RequestHandler/RequestHandler.vcxproj
+++ b/src/RequestHandler/RequestHandler.vcxproj
@@ -238,24 +238,6 @@
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
{55494e58-e061-4c4c-a0a8-837008e72f85}
@@ -270,6 +252,7 @@
+
diff --git a/src/RequestHandler/inprocess/inprocessapplication.cpp b/src/RequestHandler/inprocess/inprocessapplication.cpp
index 879ea30a63..347257d545 100644
--- a/src/RequestHandler/inprocess/inprocessapplication.cpp
+++ b/src/RequestHandler/inprocess/inprocessapplication.cpp
@@ -40,9 +40,90 @@ IN_PROCESS_APPLICATION::~IN_PROCESS_APPLICATION()
s_Application = NULL;
}
+//static
+VOID
+IN_PROCESS_APPLICATION::DoShutDown(
+ LPVOID lpParam
+)
+{
+ IN_PROCESS_APPLICATION* pApplication = static_cast(lpParam);
+ DBG_ASSERT(pApplication);
+ pApplication->ShutDownInternal();
+}
+
__override
VOID
-IN_PROCESS_APPLICATION::ShutDown()
+IN_PROCESS_APPLICATION::ShutDown(
+ VOID
+)
+{
+ HANDLE hThread = NULL;
+ HRESULT hr = S_OK;
+ DWORD dwThreadStatus = 0;
+ DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS();
+
+ if (IsDebuggerPresent())
+ {
+ dwTimeout = INFINITE;
+ }
+
+ hThread = CreateThread(
+ NULL, // default security attributes
+ 0, // default stack size
+ (LPTHREAD_START_ROUTINE)DoShutDown,
+ this, // thread function arguments
+ 0, // default creation flags
+ NULL); // receive thread identifier
+
+ if (hThread == NULL)
+ {
+ hr = HRESULT_FROM_WIN32(GetLastError());
+ goto Finished;
+ }
+
+ if (WaitForSingleObject(hThread, dwTimeout) != WAIT_OBJECT_0)
+ {
+ // if the thread is still running, we need kill it first before exit to avoid AV
+ if (GetExitCodeThread(m_hThread, &dwThreadStatus) != 0 && dwThreadStatus == STILL_ACTIVE)
+ {
+ // Calling back into managed at this point is prone to have AVs
+ // Calling terminate thread here may be our best solution.
+ TerminateThread(hThread, STATUS_CONTROL_C_EXIT);
+ hr = HRESULT_FROM_WIN32(ERROR_TIMEOUT);
+ }
+ }
+
+Finished:
+
+ if (hThread != NULL)
+ {
+ CloseHandle(hThread);
+ }
+ m_hThread = NULL;
+
+ if (FAILED(hr))
+ {
+ STACK_STRU(strEventMsg, 256);
+ //
+ // Assumption: inprocess application shutdown will be called only at process shutdown
+ // Based on this assumption, we just let shutdown continue and process will exit
+ // Log a warning for ungraceful shutdown
+ //
+ if (SUCCEEDED(strEventMsg.SafeSnwprintf(
+ ASPNETCORE_EVENT_APP_SHUTDOWN_FAILURE_MSG,
+ m_pConfig->QueryConfigPath()->QueryStr())))
+ {
+ UTILITY::LogEvent(g_hEventLog,
+ EVENTLOG_WARNING_TYPE,
+ ASPNETCORE_EVENT_GRACEFUL_SHUTDOWN_FAILURE,
+ strEventMsg.QueryStr());
+ }
+ }
+}
+
+
+VOID
+IN_PROCESS_APPLICATION::ShutDownInternal()
{
DWORD dwThreadStatus = 0;
DWORD dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS();
@@ -54,10 +135,6 @@ IN_PROCESS_APPLICATION::ShutDown()
{
dwTimeout = INFINITE;
}
- else
- {
- dwTimeout = m_pConfig->QueryShutdownTimeLimitInMS();
- }
if (m_fShutdownCalledFromNative ||
m_status == APPLICATION_STATUS::STARTING ||
diff --git a/src/RequestHandler/inprocess/inprocessapplication.h b/src/RequestHandler/inprocess/inprocessapplication.h
index 100f481e77..9de0b5255f 100644
--- a/src/RequestHandler/inprocess/inprocessapplication.h
+++ b/src/RequestHandler/inprocess/inprocessapplication.h
@@ -99,6 +99,17 @@ public:
}
private:
+ static
+ VOID
+ DoShutDown(
+ LPVOID lpParam
+ );
+
+ VOID
+ ShutDownInternal(
+ VOID
+ );
+
// Thread executing the .NET Core process
HANDLE m_hThread;
diff --git a/src/RequestHandler/outofprocess/forwardinghandler.cpp b/src/RequestHandler/outofprocess/forwardinghandler.cpp
index caf0e5d8d4..4614b27d34 100644
--- a/src/RequestHandler/outofprocess/forwardinghandler.cpp
+++ b/src/RequestHandler/outofprocess/forwardinghandler.cpp
@@ -24,7 +24,7 @@ FORWARDING_HANDLER::FORWARDING_HANDLER(
) : REQUEST_HANDLER(pW3Context, pModuleId, pApplication),
m_Signature(FORWARDING_HANDLER_SIGNATURE),
m_RequestStatus(FORWARDER_START),
- m_fHandleClosedDueToClient(FALSE),
+ m_fClientDisconnected(FALSE),
m_fResponseHeadersReceivedAndSet(FALSE),
m_fDoReverseRewriteHeaders(FALSE),
m_fFinishRequest(FALSE),
@@ -34,10 +34,18 @@ FORWARDING_HANDLER::FORWARDING_HANDLER(
m_BytesToReceive(0),
m_BytesToSend(0),
m_fWebSocketEnabled(FALSE),
- m_pWebSocket(NULL)
+ m_pWebSocket(NULL),
+ m_dwHandlers (1), // default http handler
+ m_fDoneAsyncCompletion(FALSE),
+ m_fHttpHandleInClose(FALSE),
+ m_fWebSocketHandleInClose(FALSE),
+ m_fServerResetConn(FALSE)
{
+#ifdef DEBUG
DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
"FORWARDING_HANDLER::FORWARDING_HANDLER");
+#endif
+
InitializeSRWLock(&m_RequestLock);
}
@@ -49,8 +57,10 @@ FORWARDING_HANDLER::~FORWARDING_HANDLER(
//
m_Signature = FORWARDING_HANDLER_SIGNATURE_FREE;
+#ifdef DEBUG
DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
"FORWARDING_HANDLER::~FORWARDING_HANDLER");
+#endif
//
// RemoveRequest() should already have been called and m_pDisconnect
// has been freed or m_pDisconnect was never initialized.
@@ -291,9 +301,10 @@ FORWARDING_HANDLER::OnExecuteRequestHandler()
reinterpret_cast(static_cast(this))))
{
hr = HRESULT_FROM_WIN32(GetLastError());
+#ifdef DEBUG
DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
"FORWARDING_HANDLER::OnExecuteRequestHandler, Send request failed");
-
+#endif
// FREB log
if (ANCMEvents::ANCM_REQUEST_FORWARD_FAIL::IsEnabled(m_pW3Context->GetTraceContext()))
{
@@ -398,7 +409,7 @@ REQUEST_NOTIFICATION_STATUS
BOOL fLocked = FALSE;
BOOL fClientError = FALSE;
BOOL fClosed = FALSE;
- BOOL fWebSocketUpgrade = FALSE;
+ BOOL fWebSocketUpgraded = FALSE;
DBG_ASSERT(m_pW3Context != NULL);
__analysis_assume(m_pW3Context != NULL);
@@ -421,43 +432,26 @@ REQUEST_NOTIFICATION_STATUS
if (TlsGetValue(g_dwTlsIndex) != this)
{
- DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
- AcquireSRWLockShared(&m_RequestLock);
- TlsSetValue(g_dwTlsIndex, this);
- DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
-
+ //
+ // Acquire exclusive lock as WinHTTP callback may happen on different thread
+ // We don't want two threads signal IIS pipeline simultaneously
+ //
+ AcquireLockExclusive();
fLocked = TRUE;
}
- if (m_hRequest == NULL)
+ if (m_fClientDisconnected && (m_RequestStatus != FORWARDER_DONE))
{
- // Request is Done
- if (m_fFinishRequest)
- {
- if (m_fHasError)
- {
- retVal = RQ_NOTIFICATION_FINISH_REQUEST;
- }
- else
- {
- retVal = RQ_NOTIFICATION_CONTINUE;
- }
- goto Finished;
- }
-
- fClientError = m_fHandleClosedDueToClient;
+ hr = ERROR_CONNECTION_ABORTED;
goto Failure;
}
- //
- // Begins normal completion handling. There is already a shared acquired
- // for protecting the WinHTTP request handle from being closed.
- //
- switch (m_RequestStatus)
+ if (m_RequestStatus == FORWARDER_RECEIVED_WEBSOCKET_RESPONSE)
{
- case FORWARDER_RECEIVED_WEBSOCKET_RESPONSE:
+#ifdef DEBUG
DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
"FORWARDING_HANDLER::OnAsyncCompletion, Send completed for 101 response");
+#endif
//
// This should be the write completion of the 101 response.
//
@@ -465,33 +459,49 @@ REQUEST_NOTIFICATION_STATUS
if (m_pWebSocket == NULL)
{
hr = E_OUTOFMEMORY;
- goto Finished;
+ goto Failure;
+ }
+
+ hr = m_pWebSocket->ProcessRequest(this, m_pW3Context, m_hRequest, &fWebSocketUpgraded);
+ if (fWebSocketUpgraded)
+ {
+ // WinHttp WebSocket handle has been created, bump the counter so that remember to close it
+ // and prevent from premature postcomplation and unexpected callback from winhttp
+ InterlockedIncrement(&m_dwHandlers);
}
- hr = m_pWebSocket->ProcessRequest(this, m_pW3Context, m_hRequest, &fWebSocketUpgrade);
if (FAILED(hr))
{
+ // This failure could happen when client disconnect happens or backend server fails
+ // after websocket upgrade
goto Failure;
}
//
// WebSocket upgrade is successful. Close the WinHttpRequest Handle
//
+ m_fHttpHandleInClose = TRUE;
fClosed = WinHttpCloseHandle(m_hRequest);
-
DBG_ASSERT(fClosed);
- if (fClosed)
- {
- m_hRequest = NULL;
- }
- else
+ m_hRequest = NULL;
+
+ if (!fClosed)
{
hr = HRESULT_FROM_WIN32(GetLastError());
goto Failure;
}
- break;
+ retVal = RQ_NOTIFICATION_PENDING;
+ goto Finished;
+ }
+ //
+ // Begins normal completion handling. There is already an exclusive acquired lock
+ // for protecting the WinHTTP request handle from being closed.
+ //
+ switch (m_RequestStatus)
+ {
case FORWARDER_RECEIVING_RESPONSE:
+
//
// This is a completion of a write (send) to http.sys, abort in case of
// failure, if there is more data available from WinHTTP, read it
@@ -512,6 +522,7 @@ REQUEST_NOTIFICATION_STATUS
break;
case FORWARDER_SENDING_REQUEST:
+
hr = OnSendingRequest(cbCompletion,
hrCompletionStatus,
&fClientError);
@@ -523,6 +534,23 @@ REQUEST_NOTIFICATION_STATUS
default:
DBG_ASSERT(m_RequestStatus == FORWARDER_DONE);
+ if (m_hRequest == NULL && m_pWebSocket == NULL)
+ {
+ // Request must have been done
+ if (!m_fFinishRequest)
+ {
+ goto Failure;
+ }
+
+ if (m_fHasError)
+ {
+ retVal = RQ_NOTIFICATION_FINISH_REQUEST;
+ }
+ else
+ {
+ retVal = RQ_NOTIFICATION_CONTINUE;
+ }
+ }
goto Finished;
}
@@ -536,103 +564,130 @@ REQUEST_NOTIFICATION_STATUS
Failure:
- m_fHasError = TRUE;
+ //
+ // Reset status for consistency.
+ //
m_RequestStatus = FORWARDER_DONE;
-
- //disbale client disconnect callback
- RemoveRequest();
-
- //
- // Do the right thing based on where the error originated from.
- //
- IHttpResponse *pResponse = m_pW3Context->GetResponse();
- pResponse->DisableKernelCache();
- pResponse->GetRawHttpResponse()->EntityChunkCount = 0;
-
- if (fClientError)
+ if (!m_fHasError)
{
- if (!m_fResponseHeadersReceivedAndSet)
+ m_fHasError = TRUE;
+
+ //
+ // Do the right thing based on where the error originated from.
+ //
+ IHttpResponse *pResponse = m_pW3Context->GetResponse();
+ pResponse->DisableKernelCache();
+ pResponse->GetRawHttpResponse()->EntityChunkCount = 0;
+
+ if (fClientError || m_fClientDisconnected)
{
- pResponse->SetStatus(400, "Bad Request", 0, HRESULT_FROM_WIN32(WSAECONNRESET));
+ if (!m_fResponseHeadersReceivedAndSet)
+ {
+ pResponse->SetStatus(400, "Bad Request", 0, HRESULT_FROM_WIN32(WSAECONNRESET));
+ }
+ else
+ {
+ //
+ // Response headers from origin server were
+ // already received and set for the current response.
+ // Honor the response status.
+ //
+ }
}
else
{
- //
- // Response headers from origin server were
- // already received and set for the current response.
- // Honor the response status.
- //
- }
- }
- else
- {
- STACK_STRU(strDescription, 128);
+ STACK_STRU(strDescription, 128);
- pResponse->SetStatus(502, "Bad Gateway", 3, hr);
+ pResponse->SetStatus(502, "Bad Gateway", 3, hr);
- if (!(hr > HRESULT_FROM_WIN32(WINHTTP_ERROR_BASE) &&
- hr <= HRESULT_FROM_WIN32(WINHTTP_ERROR_LAST)) ||
+ if (hr > HRESULT_FROM_WIN32(WINHTTP_ERROR_BASE) &&
+ hr <= HRESULT_FROM_WIN32(WINHTTP_ERROR_LAST))
+ {
#pragma prefast (suppress : __WARNING_FUNCTION_NEEDS_REVIEW, "Function and parameters reviewed.")
- FormatMessage(
- FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_FROM_HMODULE,
- g_hWinHttpModule,
- HRESULT_CODE(hr),
- 0,
- strDescription.QueryStr(),
- strDescription.QuerySizeCCH(),
- NULL) == 0)
- {
- LoadString(g_hAspNetCoreModule,
- IDS_SERVER_ERROR,
- strDescription.QueryStr(),
- strDescription.QuerySizeCCH());
- }
+ FormatMessage(
+ FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_FROM_HMODULE,
+ g_hWinHttpModule,
+ HRESULT_CODE(hr),
+ 0,
+ strDescription.QueryStr(),
+ strDescription.QuerySizeCCH(),
+ NULL);
+ }
+ else
+ {
+ LoadString(g_hAspNetCoreModule,
+ IDS_SERVER_ERROR,
+ strDescription.QueryStr(),
+ strDescription.QuerySizeCCH());
+ }
+ (VOID)strDescription.SyncWithBuffer();
- (VOID)strDescription.SyncWithBuffer();
- if (strDescription.QueryCCH() != 0)
- {
- pResponse->SetErrorDescription(
- strDescription.QueryStr(),
- strDescription.QueryCCH(),
- FALSE);
+ if (strDescription.QueryCCH() != 0)
+ {
+ pResponse->SetErrorDescription(
+ strDescription.QueryStr(),
+ strDescription.QueryCCH(),
+ FALSE);
+ }
+
+ if (hr == HRESULT_FROM_WIN32(ERROR_WINHTTP_INVALID_SERVER_RESPONSE))
+ {
+ if (!m_fServerResetConn)
+ {
+ RemoveRequest();
+ pResponse->ResetConnection();
+ m_fServerResetConn = TRUE;
+ }
+ }
}
}
- //
- // Finish the request on failure.
- // Let IIS pipeline continue only after receiving handle close callback
- // from WinHttp. This ensures no more callback from WinHttp
- //
- if (m_hRequest != NULL)
+ if (m_pWebSocket != NULL && !m_fWebSocketHandleInClose)
{
- if (WinHttpCloseHandle(m_hRequest))
- {
- m_hRequest = NULL;
- }
- else
- {
- // Failed to close the handle
- // which should never happen as we registered a callback with WinHttp
- // For this unexpected failure, let conitnue IIS pipeline
- /* retVal = RQ_NOTIFICATION_FINISH_REQUEST;
- DebugBreak();*/
- }
+ m_fWebSocketHandleInClose = TRUE;
+ m_pWebSocket->TerminateRequest();
+ }
+
+ if (m_hRequest != NULL && !m_fHttpHandleInClose)
+ {
+ m_fHttpHandleInClose = TRUE;
+ WinHttpCloseHandle(m_hRequest);
+ m_hRequest = NULL;
}
- retVal = RQ_NOTIFICATION_PENDING;
Finished:
+
+ if (retVal != RQ_NOTIFICATION_PENDING)
+ {
+
+ DBG_ASSERT(m_dwHandlers == 0);
+ RemoveRequest();
+
+ // This is just a safety guard to prevent from returning non pending status no more once
+ // which should never happen
+ if (!m_fDoneAsyncCompletion)
+ {
+ m_fDoneAsyncCompletion = TRUE;
+ }
+ else
+ {
+ retVal = RQ_NOTIFICATION_PENDING;
+ }
+ }
+
if (fLocked)
{
- DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
- TlsSetValue(g_dwTlsIndex, NULL);
- ReleaseSRWLockShared(&m_RequestLock);
- DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
+ ReleaseLockExclusive();
}
DereferenceRequestHandler();
//
- // No code after this point, as the handle might be gone
+ // Do not use this object after dereferencing it, it may be gone.
//
+#ifdef DEBUG
+ DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
+ "FORWARDING_HANDLER::OnAsyncCompletion Done %d", retVal);
+#endif
return retVal;
}
@@ -1037,7 +1092,6 @@ FORWARDING_HANDLER::CreateWinHttpRequest(
_In_ const PROTOCOL_CONFIG * pProtocol,
_In_ HINTERNET hConnect,
_Inout_ STRU * pstrUrl,
-// _In_ ASPNETCORE_CONFIG* pAspNetCoreConfig,
_In_ SERVER_PROCESS* pServerProcess
)
{
@@ -1206,11 +1260,14 @@ None
--*/
{
HRESULT hr = S_OK;
- BOOL fLockAcquired = FALSE;
+ BOOL fExclusiveLocked = FALSE;
+ BOOL fSharedLocked = FALSE;
BOOL fClientError = FALSE;
BOOL fAnotherCompletionExpected = FALSE;
BOOL fDoPostCompletion = FALSE;
- BOOL fEndRequest = (dwInternetStatus == WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING);
+ BOOL fHandleClosing = (dwInternetStatus == WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING);
+ DWORD dwHandlers = 1; // defaullt for http handler
+
DBG_ASSERT(m_pW3Context != NULL);
__analysis_assume(m_pW3Context != NULL);
@@ -1240,8 +1297,12 @@ None
dwInternetStatus);
}
+#ifdef DEBUG
+ DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
+ "FORWARDING_HANDLER::OnWinHttpCompletionInternal %x -- %d --%p\n", dwInternetStatus, GetCurrentThreadId(), m_pW3Context);
+#endif
//
- // ReadLock on the winhttp handle to protect from a client disconnect/
+ // Exclusive lock on the winhttp handle to protect from a client disconnect/
// server stop closing the handle while we are using it.
//
// WinHttp can call async completion on the same thread/stack, so
@@ -1252,27 +1313,39 @@ None
if (TlsGetValue(g_dwTlsIndex) != this)
{
DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
-
- AcquireSRWLockShared(&m_RequestLock);
- TlsSetValue(g_dwTlsIndex, this);
- fLockAcquired = TRUE;
- DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
+ if (m_RequestStatus != FORWARDER_RECEIVED_WEBSOCKET_RESPONSE)
+ {
+ // Webscoket has already been guarded by critical section
+ // Only require exclisive lock for non-websocket scenario which has duplex channel
+ // Otherwise, there will be a deadlock
+ AcquireLockExclusive();
+ fExclusiveLocked = TRUE;
+ }
+ else
+ {
+ AcquireSRWLockShared(&m_RequestLock);
+ TlsSetValue(g_dwTlsIndex, this);
+ fSharedLocked = TRUE;
+ DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
+ }
}
-#ifdef DEBUG
- DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
- "FORWARDING_HANDLER::OnWinHttpCompletionInternal %x -- %d --%p\n", dwInternetStatus, GetCurrentThreadId(), m_pW3Context);
-#endif // DEBUG
-
- if (!fEndRequest)
+ if (fHandleClosing)
{
- if (!m_pW3Context->GetConnection()->IsConnected())
- {
- hr = ERROR_CONNECTION_ABORTED;
- fClientError = m_fHandleClosedDueToClient = TRUE;
- fAnotherCompletionExpected = TRUE;
- goto Failure;
- }
+ dwHandlers = InterlockedDecrement(&m_dwHandlers);
+ }
+
+ if (m_fFinishRequest)
+ {
+ // Request was done by another thread, skip
+ goto Finished;
+ }
+
+
+ if (m_fClientDisconnected && (m_RequestStatus != FORWARDER_DONE))
+ {
+ hr = ERROR_CONNECTION_ABORTED;
+ goto Failure;
}
//
@@ -1373,10 +1446,10 @@ None
m_pW3Context->GetTraceContext(),
NULL);
}
- if (m_RequestStatus != FORWARDER_DONE || m_fHandleClosedDueToClient)
+ if (m_RequestStatus != FORWARDER_DONE)
{
hr = ERROR_CONNECTION_ABORTED;
- fClientError = m_fHandleClosedDueToClient;
+ fClientError = m_fClientDisconnected;
}
m_hRequest = NULL;
fAnotherCompletionExpected = FALSE;
@@ -1432,7 +1505,7 @@ Failure:
m_fResetConnection = TRUE;
}
- if (fClientError || m_fHandleClosedDueToClient)
+ if (fClientError || m_fClientDisconnected)
{
if (!m_fResponseHeadersReceivedAndSet)
{
@@ -1492,8 +1565,78 @@ Failure:
}
Finished:
+ //
+ // Since we use TLS to guard WinHttp operation, call PostCompletion instead of
+ // IndicateCompletion to allow cleaning up the TLS before thread reuse.
+ // Never post after the request has been finished for whatever reason
+ //
+ // Only postCompletion after all WinHttp handles (http and websocket) got closed,
+ // i.e., received WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING callback for both handles
+ // So that no further WinHttp callback will be called
+ // Never post completion again after that
+ // Otherwise, there will be a AV as the request already passed IIS pipeline
+ //
+ if (fHandleClosing && dwHandlers == 0)
+ {
+ //
+ // Happy path
+ //
+ // Marked the request is finished, no more PostCompletion is allowed
+ RemoveRequest();
+ m_fFinishRequest = TRUE;
+ fDoPostCompletion = TRUE;
+ if (m_pWebSocket != NULL)
+ {
+ m_pWebSocket->Terminate();
+ m_pWebSocket = NULL;
+ }
+ }
+ else if (m_RequestStatus == FORWARDER_DONE)
+ {
+ //
+ // Error path
+ //
+ RemoveRequest();
+ if (m_hRequest != NULL && !m_fHttpHandleInClose)
+ {
+ m_fHttpHandleInClose = TRUE;
+ WinHttpCloseHandle(m_hRequest);
+ m_hRequest = NULL;
+ }
- if (fLockAcquired)
+ if (m_pWebSocket != NULL && !m_fWebSocketHandleInClose)
+ {
+ m_fWebSocketHandleInClose = TRUE;
+ m_pWebSocket->TerminateRequest();
+ }
+
+ if (fHandleClosing)
+ {
+ fDoPostCompletion = dwHandlers == 0;
+ m_fFinishRequest = fDoPostCompletion;
+ }
+ }
+ else if (!fAnotherCompletionExpected)
+ {
+ //
+ // Regular async IO operation
+ //
+ fDoPostCompletion = !m_fFinishRequest;
+ }
+
+ //
+ // No code should access IIS m_pW3Context after posting the completion.
+ //
+ if (fDoPostCompletion)
+ {
+ m_pW3Context->PostCompletion(0);
+ }
+
+ if (fExclusiveLocked)
+ {
+ ReleaseLockExclusive();
+ }
+ else if (fSharedLocked)
{
DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
TlsSetValue(g_dwTlsIndex, NULL);
@@ -1501,67 +1644,8 @@ Finished:
DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
}
- if (m_RequestStatus == FORWARDER_DONE)
- {
- //disbale client disconnect callback
- RemoveRequest();
-
- if (m_hRequest != NULL)
- {
- WinHttpSetStatusCallback(m_hRequest,
- FORWARDING_HANDLER::OnWinHttpCompletion,
- WINHTTP_CALLBACK_FLAG_HANDLES,
- NULL);
- if (WinHttpCloseHandle(m_hRequest))
- {
- m_hRequest = NULL;
- }
- else
- {
- // unexpected WinHttp error, log it
- /*DebugBreak();
- m_RequestStatus = FORWARDER_FINISH_REQUEST;
- fDoPostCompletion = TRUE;*/
- }
- }
-
- //
- // If the request is a websocket request, initiate cleanup.
- //
- if (m_pWebSocket != NULL)
- {
- m_pWebSocket->TerminateRequest();
- }
-
- if (fEndRequest)
- {
- // only postCompletion after WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING
- // so that no further WinHttp callback will be called
- // in case of websocket, m_hRequest has already been closed after upgrade
- // websocket will handle completion
- m_fFinishRequest = TRUE;
- fDoPostCompletion = TRUE;
- }
- }
- //
- // Completion may had been already posted to IIS if an async
- // operation was started in this method (either WinHTTP or IIS e.g. ReadyEntityBody)
- // If fAnotherCompletionExpected is false, this method must post the completion.
- //
- else if (!fAnotherCompletionExpected)
- {
- //
- // Since we use TLS to guard WinHttp operation, call PostCompletion instead of
- // IndicateCompletion to allow cleaning up the TLS before thread reuse.
- //
- fDoPostCompletion = TRUE;
- }
-
DereferenceRequestHandler();
- if (fDoPostCompletion)
- {
- m_pW3Context->PostCompletion(0);
- }
+
}
HRESULT
@@ -2612,33 +2696,52 @@ FORWARDING_HANDLER::TerminateRequest(
)
{
UNREFERENCED_PARAMETER(fClientInitiated);
- AcquireSRWLockExclusive(&m_RequestLock);
+
+ BOOL fLocked = FALSE;
+ if (TlsGetValue(g_dwTlsIndex) != this)
+ {
+ //
+ // Acquire exclusive lock as WinHTTP callback may happen on different thread
+ // We don't want two threads signal IIS pipeline simultaneously
+ //
+ AcquireLockExclusive();
+ fLocked = TRUE;
+ }
+
// Set tls as close winhttp handle will immediately trigger
// a winhttp callback on the same thread and we donot want to
// acquire the lock again
+
+#ifdef DEBUG
+ DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
+ "FORWARDING_HANDLER::TerminateRequest %d --%p\n", GetCurrentThreadId(), m_pW3Context);
+#endif // DEBUG
+
+ if (!m_fHttpHandleInClose)
+ {
+ m_fClientDisconnected = fClientInitiated;
+ }
+
+ if (fLocked)
+ {
+ ReleaseLockExclusive();
+ }
+}
+
+VOID
+FORWARDING_HANDLER::AcquireLockExclusive()
+{
+ DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
+ AcquireSRWLockExclusive(&m_RequestLock);
TlsSetValue(g_dwTlsIndex, this);
DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
+}
- if (m_hRequest != NULL)
- {
-#ifdef DEBUG
- DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO,
- "FORWARDING_HANDLER::TerminateRequest %d --%p\n", GetCurrentThreadId(), m_pW3Context);
-#endif // DEBUG
- m_fHandleClosedDueToClient = fClientInitiated;
- WinHttpCloseHandle(m_hRequest);
- }
-
- //
- // If the request is a websocket request, initiate cleanup.
- //
- if (m_pWebSocket != NULL)
- {
- m_pWebSocket->TerminateRequest();
- }
-
+VOID
+FORWARDING_HANDLER::ReleaseLockExclusive()
+{
+ DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this);
TlsSetValue(g_dwTlsIndex, NULL);
ReleaseSRWLockExclusive(&m_RequestLock);
DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == NULL);
}
-
diff --git a/src/RequestHandler/outofprocess/forwardinghandler.h b/src/RequestHandler/outofprocess/forwardinghandler.h
index efdcb35ed9..427540f2d3 100644
--- a/src/RequestHandler/outofprocess/forwardinghandler.h
+++ b/src/RequestHandler/outofprocess/forwardinghandler.h
@@ -73,13 +73,19 @@ public:
);
private:
+
+ VOID
+ AcquireLockExclusive();
+
+ VOID
+ ReleaseLockExclusive();
+
HRESULT
CreateWinHttpRequest(
_In_ const IHttpRequest * pRequest,
_In_ const PROTOCOL_CONFIG * pProtocol,
_In_ HINTERNET hConnect,
_Inout_ STRU * pstrUrl,
-// _In_ ASPNETCORE_CONFIG* pAspNetCoreConfig,
_In_ SERVER_PROCESS* pServerProcess
);
@@ -173,12 +179,33 @@ private:
BOOL m_fWebSocketEnabled;
BOOL m_fResponseHeadersReceivedAndSet;
BOOL m_fResetConnection;
- BOOL m_fHandleClosedDueToClient;
- BOOL m_fFinishRequest;
- BOOL m_fHasError;
BOOL m_fDoReverseRewriteHeaders;
+ BOOL m_fServerResetConn;
+ volatile BOOL m_fClientDisconnected;
+ //
+ // A safety guard flag indicating no more IIS PostCompletion is allowed
+ //
+ volatile BOOL m_fFinishRequest;
+ //
+ // A safety guard flag to prevent from unexpect callback which may signal IIS pipeline
+ // more than once with non-pending status
+ //
+ volatile BOOL m_fDoneAsyncCompletion;
+ volatile BOOL m_fHasError;
+ //
+ // WinHttp may hit AV under race if handle got closed more than once simultaneously
+ // Use two bool variables to guard
+ //
+ volatile BOOL m_fHttpHandleInClose;
+ volatile BOOL m_fWebSocketHandleInClose;
+
PCSTR m_pszOriginalHostHeader;
PCWSTR m_pszHeaders;
+ //
+ // Record the number of winhttp handles in use
+ // release IIS pipeline only after all handles got closed
+ //
+ volatile LONG m_dwHandlers;
DWORD m_cchHeaders;
DWORD m_BytesToReceive;
DWORD m_BytesToSend;
diff --git a/src/RequestHandler/requesthandler.rc b/src/RequestHandler/requesthandler.rc
index 1451904d47..2cc99c2331 100644
--- a/src/RequestHandler/requesthandler.rc
+++ b/src/RequestHandler/requesthandler.rc
@@ -9,6 +9,8 @@
#if !defined(AFX_RESOURCE_DLL) || defined(AFX_TARG_ENU)
LANGUAGE LANG_ENGLISH, SUBLANG_ENGLISH_US
+#define FileDescription "IIS ASP.NET Core Module Request Handler. Commit: " CommitHash
+
/////////////////////////////////////////////////////////////////////////////
//
// 11
@@ -74,7 +76,7 @@ BEGIN
BLOCK "040904b0"
BEGIN
VALUE "CompanyName", "Microsoft"
- VALUE "FileDescription", "IIS ASP.NET Core Module Request Handler"
+ VALUE "FileDescription", FileDescription
VALUE "FileVersion", FileVersionStr
VALUE "InternalName", "aspnetcorerh.dll"
VALUE "LegalCopyright", "Copyright (C) Microsoft Corporation"