fix #275: refactor graceful recycling logic to avoid deadlock potential

tjanczuk · May 3, 2013 · 58c2898 · 58c2898
1 parent a3f0db9
commit 58c2898
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 4 deletions.
diff --git a/src/iisnode/cnodeapplication.cpp b/src/iisnode/cnodeapplication.cpp
@@ -2,7 +2,7 @@
 
 CNodeApplication::CNodeApplication(CNodeApplicationManager* applicationManager, BOOL isDebugger, NodeDebugCommand debugCommand, DWORD debugPort)
 	: applicationManager(applicationManager), scriptName(NULL), processManager(NULL),
-	isDebugger(isDebugger), peerApplication(NULL), debugCommand(debugCommand), debugPort(debugPort)
+	isDebugger(isDebugger), peerApplication(NULL), debugCommand(debugCommand), debugPort(debugPort), needsRecycling(FALSE)
 {
 }
 
@@ -146,3 +146,14 @@ DWORD CNodeApplication::GetProcessCount()
 {
 	return this->processManager->GetProcessCount();
 }
+
+void CNodeApplication::SetNeedsRecycling()
+{
+	this->needsRecycling = TRUE;
+}
+
+BOOL CNodeApplication::GetNeedsRecycling()
+{
+	return this->needsRecycling;
+}
+
diff --git a/src/iisnode/cnodeapplication.h b/src/iisnode/cnodeapplication.h
@@ -17,6 +17,7 @@ class CNodeApplication
 	BOOL isDebugger;
 	NodeDebugCommand debugCommand;
 	DWORD debugPort;
+	BOOL needsRecycling;
 
 	void Cleanup();	
 
@@ -39,6 +40,8 @@ class CNodeApplication
 	DWORD GetDebugPort();
 	DWORD GetActiveRequestCount();
 	DWORD GetProcessCount();
+	void SetNeedsRecycling();
+	BOOL GetNeedsRecycling();
 };
 
 #endif
diff --git a/src/iisnode/cnodeapplicationmanager.cpp b/src/iisnode/cnodeapplicationmanager.cpp
@@ -193,7 +193,7 @@ HRESULT CNodeApplicationManager::Dispatch(IHttpContext* context, IHttpEventProvi
         ENTER_SRW_SHARED(this->srwlock)
 
         CheckError(this->GetOrCreateNodeApplication(context, debugCommand, FALSE, &application));
-		if (application)
+		if (application && !application->GetNeedsRecycling())
 		{
 			// this is the sweetspot code path: application already exists, shared read lock is sufficient
 
@@ -202,14 +202,20 @@ HRESULT CNodeApplicationManager::Dispatch(IHttpContext* context, IHttpEventProvi
 
         LEAVE_SRW_SHARED(this->srwlock)
 
-		if (!application)
+		if (!application || application->GetNeedsRecycling())
 		{
 			// this is the initialization code path for activating request:
 			// application must be created which requires an exclusive lock
 
 			ENTER_SRW_EXCLUSIVE(this->srwlock)
 
 			CheckError(this->GetOrCreateNodeApplication(context, debugCommand, TRUE, &application));
+			if (application->GetNeedsRecycling())
+			{
+				this->RecycleApplicationAssumeLock(application);
+				CheckError(this->GetOrCreateNodeApplication(context, debugCommand, TRUE, &application));
+			}
+
 			CheckError(application->Dispatch(context, pProvider, ctx));
 
 			LEAVE_SRW_EXCLUSIVE(this->srwlock)
@@ -339,7 +345,7 @@ HRESULT CNodeApplicationManager::RecycleApplicationAssumeLock(CNodeApplication*
 
 void CNodeApplicationManager::OnScriptModified(CNodeApplicationManager* manager, CNodeApplication* application)
 {
-    manager->RecycleApplication(application);
+    application->SetNeedsRecycling();
 }
 
 // this method is always called under exclusive this->srwlock