Skip to content

Commit 19e750e

Browse files
committedNov 15, 2023
OrcLib: CommandExecute: add CreateChildProcess and ResumeChildProcess
Split method Execute to be able to handle child process attributes before starting execution and avoid some race condition.
1 parent c47d3f2 commit 19e750e

File tree

3 files changed

+44
-13
lines changed

3 files changed

+44
-13
lines changed
 

‎src/OrcLib/CommandAgent.cpp

+15-12
Original file line numberDiff line numberDiff line change
@@ -777,14 +777,17 @@ HRESULT CommandAgent::ExecuteNextCommand()
777777
m_MaximumRunningSemaphore.Release();
778778
}
779779
}
780+
780781
if (command)
781782
{
782-
// there is something to execute in the queue
783-
784-
hr = command->Execute(m_Job, m_bWillRequireBreakAway);
785-
783+
hr = command->CreateChildProcess(m_Job, m_bWillRequireBreakAway);
786784
if (SUCCEEDED(hr))
787785
{
786+
{
787+
Concurrency::critical_section::scoped_lock s(m_cs);
788+
m_RunningCommands.push_back(command);
789+
}
790+
788791
if (command->GetTimeout().has_value() && command->GetTimeout()->count() != 0)
789792
{
790793
auto timer = std::make_shared<Concurrency::timer<CommandMessage::Message>>(
@@ -797,15 +800,8 @@ HRESULT CommandAgent::ExecuteNextCommand()
797800
timer->start();
798801
}
799802

800-
{
801-
Concurrency::critical_section::scoped_lock s(m_cs);
802-
m_RunningCommands.push_back(command);
803-
}
804-
805803
HANDLE hWaitObject = INVALID_HANDLE_VALUE;
806-
807804
CompletionBlock* pBlockPtr = (CompletionBlock*)Concurrency::Alloc(sizeof(CompletionBlock));
808-
809805
CompletionBlock* pBlock = new (pBlockPtr) CompletionBlock;
810806
pBlock->pAgent = this;
811807
pBlock->command = command;
@@ -824,6 +820,14 @@ HRESULT CommandAgent::ExecuteNextCommand()
824820
return hr;
825821
}
826822

823+
hr = command->ResumeChildProcess();
824+
if (FAILED(hr))
825+
{
826+
m_MaximumRunningSemaphore.Release();
827+
command->CompleteExecution();
828+
return S_OK;
829+
}
830+
827831
auto notification = CommandNotification::NotifyStarted(
828832
command->ProcessID(), command->GetKeyword(), command->m_pi.hProcess, command->m_commandLine);
829833

@@ -837,7 +841,6 @@ HRESULT CommandAgent::ExecuteNextCommand()
837841
}
838842
else
839843
{
840-
// Process execution failed to start, releasing semaphone
841844
m_MaximumRunningSemaphore.Release();
842845
command->CompleteExecution();
843846
}

‎src/OrcLib/CommandExecute.cpp

+26-1
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,23 @@ HRESULT CommandExecute::AddDumpFileDirectory(const std::wstring& strDirectory)
260260
}
261261

262262
HRESULT CommandExecute::Execute(const JobObject& job, bool bBreakAway)
263+
{
264+
HRESULT hr = CreateChildProcess(job, bBreakAway);
265+
if (FAILED(hr))
266+
{
267+
return hr;
268+
}
269+
270+
hr = ResumeChildProcess();
271+
if (FAILED(hr))
272+
{
273+
return hr;
274+
}
275+
276+
return S_OK;
277+
}
278+
279+
HRESULT CommandExecute::CreateChildProcess(const JobObject& job, bool bBreakAway)
263280
{
264281
HRESULT hr = E_FAIL;
265282
wstring cmdLineBuilder;
@@ -396,9 +413,17 @@ HRESULT CommandExecute::Execute(const JobObject& job, bool bBreakAway)
396413
// Command as 'GetSamples' will create a child process 'GetThis' with the current log file path for appending
397414
Log::Flush();
398415

416+
SetStatus(Created);
417+
return S_OK;
418+
}
419+
420+
HRESULT CommandExecute::ResumeChildProcess()
421+
{
422+
assert(GetStatus() == Created);
423+
399424
if (ResumeThread(m_pi.hThread) == -1)
400425
{
401-
hr = HRESULT_FROM_WIN32(GetLastError());
426+
HRESULT hr = HRESULT_FROM_WIN32(GetLastError());
402427
Log::Error(L"Failed to resume process '{}' [{}]", m_Keyword, SystemError(hr));
403428
TerminateProcess(m_pi.hProcess, (UINT)-1);
404429
return hr;

‎src/OrcLib/CommandExecute.h

+3
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ class CommandExecute
152152
typedef enum _ProcessStatus
153153
{
154154
Initialized,
155+
Created,
155156
Started,
156157
Complete,
157158
Closed
@@ -175,6 +176,8 @@ class CommandExecute
175176
HRESULT AddDumpFileDirectory(const std::wstring& strDirectory);
176177

177178
HRESULT Execute(const JobObject& job, bool bBreakAway = true);
179+
HRESULT CreateChildProcess(const JobObject& job, bool bBreakAway = true);
180+
HRESULT ResumeChildProcess();
178181

179182
HANDLE ProcessHandle() { return m_pi.hProcess; };
180183

0 commit comments

Comments
 (0)
Please sign in to comment.