Don't call CloseHandle() twice on Windows (as it causes crashes, or unexpected behavior). This would happen with the following test case:

ProcessHandle handle = Process.launch(...);
handle.kill();
Then as handle gets out of scope, ~ProcessHandle would call CloseHandle() on an already closed handle.
This commit is contained in:
Patrice Tarabbia
2013-05-01 06:41:45 -04:00
parent e06fec0e24
commit c59f8865ac
14 changed files with 67 additions and 33 deletions

View File

@@ -237,7 +237,7 @@ public:
/// Waits for the process specified by handle to terminate /// Waits for the process specified by handle to terminate
/// and returns the exit code of the process. /// and returns the exit code of the process.
static void kill(const ProcessHandle& handle); static void kill(ProcessHandle& handle);
/// Kills the process specified by handle. /// Kills the process specified by handle.
/// ///
/// This is preferable on Windows where process IDs /// This is preferable on Windows where process IDs

View File

@@ -84,7 +84,7 @@ public:
Pipe* outPipe, Pipe* outPipe,
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);

View File

@@ -85,7 +85,7 @@ public:
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static int waitImpl(PIDImpl pid); static int waitImpl(PIDImpl pid);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);
}; };

View File

@@ -86,7 +86,7 @@ public:
Pipe* outPipe, Pipe* outPipe,
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);
}; };

View File

@@ -62,6 +62,7 @@ public:
UInt32 id() const; UInt32 id() const;
HANDLE process() const; HANDLE process() const;
int wait() const; int wait() const;
void closeHandle();
private: private:
HANDLE _hProcess; HANDLE _hProcess;
@@ -89,7 +90,7 @@ public:
Pipe* outPipe, Pipe* outPipe,
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);
static std::string terminationEventName(PIDImpl pid); static std::string terminationEventName(PIDImpl pid);

View File

@@ -62,6 +62,7 @@ public:
UInt32 id() const; UInt32 id() const;
HANDLE process() const; HANDLE process() const;
int wait() const; int wait() const;
void closeHandle();
private: private:
HANDLE _hProcess; HANDLE _hProcess;
@@ -89,7 +90,7 @@ public:
Pipe* outPipe, Pipe* outPipe,
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);
static std::string terminationEventName(PIDImpl pid); static std::string terminationEventName(PIDImpl pid);

View File

@@ -62,6 +62,7 @@ public:
UInt32 id() const; UInt32 id() const;
HANDLE process() const; HANDLE process() const;
int wait() const; int wait() const;
void closeHandle();
private: private:
HANDLE _hProcess; HANDLE _hProcess;
@@ -89,7 +90,7 @@ public:
Pipe* outPipe, Pipe* outPipe,
Pipe* errPipe, Pipe* errPipe,
const EnvImpl& env); const EnvImpl& env);
static void killImpl(const ProcessHandleImpl& handle); static void killImpl(ProcessHandleImpl& handle);
static void killImpl(PIDImpl pid); static void killImpl(PIDImpl pid);
static void requestTerminationImpl(PIDImpl pid); static void requestTerminationImpl(PIDImpl pid);
static std::string terminationEventName(PIDImpl pid); static std::string terminationEventName(PIDImpl pid);

View File

@@ -200,7 +200,7 @@ int Process::wait(const ProcessHandle& handle)
} }
void Process::kill(const ProcessHandle& handle) void Process::kill(ProcessHandle& handle)
{ {
killImpl(*handle._pImpl); killImpl(*handle._pImpl);
} }

View File

@@ -218,7 +218,7 @@ ProcessHandleImpl* ProcessImpl::launchByForkExecImpl(const std::string& command,
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
killImpl(handle.id()); killImpl(handle.id());
} }

View File

@@ -137,7 +137,7 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
killImpl(handle.id()); killImpl(handle.id());
} }

View File

@@ -89,7 +89,7 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
throw Poco::NotImplementedException("Process::kill()"); throw Poco::NotImplementedException("Process::kill()");
} }

View File

@@ -56,9 +56,17 @@ ProcessHandleImpl::ProcessHandleImpl(HANDLE hProcess, UInt32 pid):
ProcessHandleImpl::~ProcessHandleImpl() ProcessHandleImpl::~ProcessHandleImpl()
{ {
CloseHandle(_hProcess); closeHandle();
} }
void ProcessHandleImpl::closeHandle()
{
if (_hProcess)
{
CloseHandle(_hProcess);
_hProcess = NULL;
}
}
UInt32 ProcessHandleImpl::id() const UInt32 ProcessHandleImpl::id() const
{ {
@@ -228,17 +236,19 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
if (handle.process())
{
if (TerminateProcess(handle.process(), 0) == 0) if (TerminateProcess(handle.process(), 0) == 0)
{ {
CloseHandle(handle.process()); handle.closeHandle();
throw SystemException("cannot kill process"); throw SystemException("cannot kill process");
} }
CloseHandle(handle.process()); handle.closeHandle();
}
} }
void ProcessImpl::killImpl(PIDImpl pid) void ProcessImpl::killImpl(PIDImpl pid)
{ {
HANDLE hProc = OpenProcess(PROCESS_TERMINATE, FALSE, pid); HANDLE hProc = OpenProcess(PROCESS_TERMINATE, FALSE, pid);

View File

@@ -57,9 +57,17 @@ ProcessHandleImpl::ProcessHandleImpl(HANDLE hProcess, UInt32 pid):
ProcessHandleImpl::~ProcessHandleImpl() ProcessHandleImpl::~ProcessHandleImpl()
{ {
CloseHandle(_hProcess); closeHandle();
} }
void ProcessHandleImpl::closeHandle()
{
if (_hProcess)
{
CloseHandle(_hProcess);
_hProcess = NULL;
}
}
UInt32 ProcessHandleImpl::id() const UInt32 ProcessHandleImpl::id() const
{ {
@@ -234,17 +242,19 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
if (handle.process())
{
if (TerminateProcess(handle.process(), 0) == 0) if (TerminateProcess(handle.process(), 0) == 0)
{ {
CloseHandle(handle.process()); handle.closeHandle();
throw SystemException("cannot kill process"); throw SystemException("cannot kill process");
} }
CloseHandle(handle.process()); handle.closeHandle();
}
} }
void ProcessImpl::killImpl(PIDImpl pid) void ProcessImpl::killImpl(PIDImpl pid)
{ {
HANDLE hProc = OpenProcess(PROCESS_TERMINATE, FALSE, pid); HANDLE hProc = OpenProcess(PROCESS_TERMINATE, FALSE, pid);

View File

@@ -57,9 +57,17 @@ ProcessHandleImpl::ProcessHandleImpl(HANDLE hProcess, UInt32 pid):
ProcessHandleImpl::~ProcessHandleImpl() ProcessHandleImpl::~ProcessHandleImpl()
{ {
CloseHandle(_hProcess); closeHandle();
} }
void ProcessHandleImpl::closeHandle()
{
if (_hProcess)
{
CloseHandle(_hProcess);
_hProcess = NULL;
}
}
UInt32 ProcessHandleImpl::id() const UInt32 ProcessHandleImpl::id() const
{ {
@@ -158,14 +166,17 @@ ProcessHandleImpl* ProcessImpl::launchImpl(const std::string& command, const Arg
} }
void ProcessImpl::killImpl(const ProcessHandleImpl& handle) void ProcessImpl::killImpl(ProcessHandleImpl& handle)
{ {
if (handle.process())
{
if (TerminateProcess(handle.process(), 0) == 0) if (TerminateProcess(handle.process(), 0) == 0)
{ {
CloseHandle(handle.process()); handle.closeHandle();
throw SystemException("cannot kill process"); throw SystemException("cannot kill process");
} }
CloseHandle(handle.process()); handle.closeHandle();
}
} }