Skip to content

Commit 58c2898

Browse files
committed
fix #275: refactor graceful recycling logic to avoid deadlock potential
1 parent a3f0db9 commit 58c2898

File tree

3 files changed

+24
-4
lines changed

3 files changed

+24
-4
lines changed

Diff for: src/iisnode/cnodeapplication.cpp

+12-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
CNodeApplication::CNodeApplication(CNodeApplicationManager* applicationManager, BOOL isDebugger, NodeDebugCommand debugCommand, DWORD debugPort)
44
: applicationManager(applicationManager), scriptName(NULL), processManager(NULL),
5-
isDebugger(isDebugger), peerApplication(NULL), debugCommand(debugCommand), debugPort(debugPort)
5+
isDebugger(isDebugger), peerApplication(NULL), debugCommand(debugCommand), debugPort(debugPort), needsRecycling(FALSE)
66
{
77
}
88

@@ -146,3 +146,14 @@ DWORD CNodeApplication::GetProcessCount()
146146
{
147147
return this->processManager->GetProcessCount();
148148
}
149+
150+
void CNodeApplication::SetNeedsRecycling()
151+
{
152+
this->needsRecycling = TRUE;
153+
}
154+
155+
BOOL CNodeApplication::GetNeedsRecycling()
156+
{
157+
return this->needsRecycling;
158+
}
159+

Diff for: src/iisnode/cnodeapplication.h

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ class CNodeApplication
1717
BOOL isDebugger;
1818
NodeDebugCommand debugCommand;
1919
DWORD debugPort;
20+
BOOL needsRecycling;
2021

2122
void Cleanup();
2223

@@ -39,6 +40,8 @@ class CNodeApplication
3940
DWORD GetDebugPort();
4041
DWORD GetActiveRequestCount();
4142
DWORD GetProcessCount();
43+
void SetNeedsRecycling();
44+
BOOL GetNeedsRecycling();
4245
};
4346

4447
#endif

Diff for: src/iisnode/cnodeapplicationmanager.cpp

+9-3
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ HRESULT CNodeApplicationManager::Dispatch(IHttpContext* context, IHttpEventProvi
193193
ENTER_SRW_SHARED(this->srwlock)
194194

195195
CheckError(this->GetOrCreateNodeApplication(context, debugCommand, FALSE, &application));
196-
if (application)
196+
if (application && !application->GetNeedsRecycling())
197197
{
198198
// this is the sweetspot code path: application already exists, shared read lock is sufficient
199199

@@ -202,14 +202,20 @@ HRESULT CNodeApplicationManager::Dispatch(IHttpContext* context, IHttpEventProvi
202202

203203
LEAVE_SRW_SHARED(this->srwlock)
204204

205-
if (!application)
205+
if (!application || application->GetNeedsRecycling())
206206
{
207207
// this is the initialization code path for activating request:
208208
// application must be created which requires an exclusive lock
209209

210210
ENTER_SRW_EXCLUSIVE(this->srwlock)
211211

212212
CheckError(this->GetOrCreateNodeApplication(context, debugCommand, TRUE, &application));
213+
if (application->GetNeedsRecycling())
214+
{
215+
this->RecycleApplicationAssumeLock(application);
216+
CheckError(this->GetOrCreateNodeApplication(context, debugCommand, TRUE, &application));
217+
}
218+
213219
CheckError(application->Dispatch(context, pProvider, ctx));
214220

215221
LEAVE_SRW_EXCLUSIVE(this->srwlock)
@@ -339,7 +345,7 @@ HRESULT CNodeApplicationManager::RecycleApplicationAssumeLock(CNodeApplication*
339345

340346
void CNodeApplicationManager::OnScriptModified(CNodeApplicationManager* manager, CNodeApplication* application)
341347
{
342-
manager->RecycleApplication(application);
348+
application->SetNeedsRecycling();
343349
}
344350

345351
// this method is always called under exclusive this->srwlock

0 commit comments

Comments
 (0)