#6567 Fixed memory leak causing zombie handles to created in the deamon on windows

Capture all process information as they are created to properly kill off handles when process are stopped, also added in a kill everything when the program cleans up all synergy(s|c) processes
This commit is contained in:
Jamie Newbon 2020-09-08 12:15:06 +01:00
parent af63aab1e0
commit f25947e68b
2 changed files with 63 additions and 15 deletions

View file

@ -37,6 +37,7 @@
#include <UserEnv.h> #include <UserEnv.h>
#include <Shellapi.h> #include <Shellapi.h>
#define CURRENT_PROCESS_ID 0
#define MAXIMUM_WAIT_TIME 3 #define MAXIMUM_WAIT_TIME 3
enum { enum {
kOutputBufferSize = 4096 kOutputBufferSize = 4096
@ -338,6 +339,7 @@ MSWindowsWatchdog::startProcess()
// wait for program to fail. // wait for program to fail.
ARCH->sleep(1); ARCH->sleep(1);
if (!isProcessActive()) { if (!isProcessActive()) {
closeProcessHandles(m_processInfo.dwProcessId);
throw XMSWindowsWatchdogError("process immediately stopped"); throw XMSWindowsWatchdogError("process immediately stopped");
} }
@ -376,9 +378,13 @@ MSWindowsWatchdog::startProcessInForeground(String& command)
si.dwFlags |= STARTF_USESHOWWINDOW; si.dwFlags |= STARTF_USESHOWWINDOW;
si.wShowWindow = SW_MINIMIZE; si.wShowWindow = SW_MINIMIZE;
return CreateProcess( BOOL result = CreateProcess(
NULL, LPSTR(command.c_str()), NULL, NULL, NULL, LPSTR(command.c_str()), NULL, NULL,
TRUE, 0, NULL, NULL, &si, &m_processInfo); TRUE, 0, NULL, NULL, &si, &m_processInfo);
m_children.insert(std::make_pair(m_processInfo.dwProcessId, m_processInfo));
return result;
} }
BOOL BOOL
@ -409,6 +415,8 @@ MSWindowsWatchdog::startProcessAsUser(String& command, HANDLE userToken, LPSECUR
sa, NULL, TRUE, creationFlags, sa, NULL, TRUE, creationFlags,
environment, NULL, &si, &m_processInfo); environment, NULL, &si, &m_processInfo);
m_children.insert(std::make_pair(m_processInfo.dwProcessId, m_processInfo));
DestroyEnvironmentBlock(environment); DestroyEnvironmentBlock(environment);
CloseHandle(userToken); CloseHandle(userToken);
@ -529,13 +537,15 @@ MSWindowsWatchdog::shutdownProcess(HANDLE handle, DWORD pid, int timeout)
ARCH->sleep(1); ARCH->sleep(1);
} }
} }
closeProcessHandles(pid);
} }
void void
MSWindowsWatchdog::shutdownExistingProcesses() MSWindowsWatchdog::shutdownExistingProcesses()
{ {
// first we need to take a snapshot of the running processes // first we need to take a snapshot of the running processes
HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, CURRENT_PROCESS_ID);
if (snapshot == INVALID_HANDLE_VALUE) { if (snapshot == INVALID_HANDLE_VALUE) {
LOG((CLOG_ERR "could not get process snapshot")); LOG((CLOG_ERR "could not get process snapshot"));
throw XArch(new XArchEvalWindows); throw XArch(new XArchEvalWindows);
@ -581,6 +591,7 @@ MSWindowsWatchdog::shutdownExistingProcesses()
} }
} }
clearAllChildren();
CloseHandle(snapshot); CloseHandle(snapshot);
m_processRunning = false; m_processRunning = false;
} }
@ -601,7 +612,7 @@ MSWindowsWatchdog::getActiveDesktop(LPSECURITY_ATTRIBUTES security)
m_elevateProcess = elevateProcess; m_elevateProcess = elevateProcess;
BOOL createRet = startProcessAsUser(syntoolCommand, userToken, security); BOOL createRet = startProcessAsUser(syntoolCommand, userToken, security);
auto pid = m_processInfo.dwProcessId;
if (!createRet) { if (!createRet) {
DWORD rc = GetLastError(); DWORD rc = GetLastError();
RevertToSelf(); RevertToSelf();
@ -622,6 +633,7 @@ MSWindowsWatchdog::getActiveDesktop(LPSECURITY_ATTRIBUTES security)
} }
m_ready = false; m_ready = false;
ARCH->unlockMutex(m_mutex); ARCH->unlockMutex(m_mutex);
closeProcessHandles(pid);
} }
} }
@ -644,3 +656,22 @@ MSWindowsWatchdog::testOutput(String buffer)
ARCH->unlockMutex(m_mutex); ARCH->unlockMutex(m_mutex);
} }
} }
void MSWindowsWatchdog::closeProcessHandles(unsigned long pid, bool removeFromMap) {
auto processInfo = m_children.find(pid);
if (processInfo != m_children.end()) {
CloseHandle(processInfo->second.hProcess);
CloseHandle(processInfo->second.hThread);
if(removeFromMap) {
m_children.erase(processInfo);
}
}
}
void MSWindowsWatchdog::clearAllChildren() {
for (auto it = m_children.begin(); it != m_children.end(); ++it)
{
closeProcessHandles(it->second.dwThreadId, false);
}
m_children.clear();
}

View file

@ -26,6 +26,7 @@
#include <Windows.h> #include <Windows.h>
#include <string> #include <string>
#include <list> #include <list>
#include <map>
class Thread; class Thread;
class IpcLogOutputter; class IpcLogOutputter;
@ -62,6 +63,17 @@ private:
void getActiveDesktop(LPSECURITY_ATTRIBUTES security); void getActiveDesktop(LPSECURITY_ATTRIBUTES security);
void testOutput(String buffer); void testOutput(String buffer);
void setStartupInfo(STARTUPINFO& si); void setStartupInfo(STARTUPINFO& si);
void checkChildren();
/**
* @brief This closes the handles held to a child thread
* @param pid the ID of the process to kill, will do nothing if PID is not a valid child
* @param removeFromMap should the function remove the item from the children map
*/
void closeProcessHandles(unsigned long pid, bool removeFromMap = true);
/**
* @brief This kills off all children's handles created by this process
*/
void clearAllChildren();
private: private:
Thread* m_thread; Thread* m_thread;
@ -85,6 +97,11 @@ private:
ArchCond m_condVar; ArchCond m_condVar;
bool m_ready; bool m_ready;
bool m_foreground; bool m_foreground;
/// @brief Save the info of all process made
/// We will use this to track all processes we make and
/// kill off handels and children that we no longer need
std::map<unsigned long, PROCESS_INFORMATION> m_children;
}; };
//! Relauncher error //! Relauncher error