From ea45d7dcd8cb3c2bf039997ddc3f6a0a846a2b7b Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 3 Jul 2021 11:01:09 +0100 Subject: [PATCH] Close all thread handles returned from CreateThread. If you don't, they are permanently leaked. A user points out that this is particularly bad in Pageant, with the new named-pipe-based IPC, since it will spawn an input and output I/O thread per named pipe connection, leading to two handles being leaked every time. (cherry picked from commit c714dfc936285fb588d047bb71b27cd396af6490) --- windows/window.c | 6 ++++-- windows/winhandl.c | 12 ++++++++---- windows/winpgen.c | 6 ++++-- windows/winpgnt.c | 6 ++++-- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/windows/window.c b/windows/window.c index 0c92a8f5..a37f6525 100644 --- a/windows/window.c +++ b/windows/window.c @@ -5440,8 +5440,10 @@ static void wintw_clip_request_paste(TermWin *tw, int clipboard) * that tells us it's OK to paste. */ DWORD in_threadid; /* required for Win9x */ - CreateThread(NULL, 0, clipboard_read_threadfunc, - wgs.term_hwnd, 0, &in_threadid); + HANDLE hThread = CreateThread(NULL, 0, clipboard_read_threadfunc, + wgs.term_hwnd, 0, &in_threadid); + if (hThread) + CloseHandle(hThread); /* we don't need the thread handle */ } /* diff --git a/windows/winhandl.c b/windows/winhandl.c index 085f30a0..82d2aded 100644 --- a/windows/winhandl.c +++ b/windows/winhandl.c @@ -451,8 +451,10 @@ struct handle *handle_input_new(HANDLE handle, handle_inputfn_t gotdata, handles_by_evtomain = newtree234(handle_cmp_evtomain); add234(handles_by_evtomain, h); - CreateThread(NULL, 0, handle_input_threadfunc, - &h->u.i, 0, &in_threadid); + HANDLE hThread = CreateThread(NULL, 0, handle_input_threadfunc, + &h->u.i, 0, &in_threadid); + if (hThread) + CloseHandle(hThread); /* we don't need the thread handle */ h->u.i.busy = true; return h; @@ -482,8 +484,10 @@ struct handle *handle_output_new(HANDLE handle, handle_outputfn_t sentdata, handles_by_evtomain = newtree234(handle_cmp_evtomain); add234(handles_by_evtomain, h); - CreateThread(NULL, 0, handle_output_threadfunc, - &h->u.o, 0, &out_threadid); + HANDLE hThread = CreateThread(NULL, 0, handle_output_threadfunc, + &h->u.o, 0, &out_threadid); + if (hThread) + CloseHandle(hThread); /* we don't need the thread handle */ return h; } diff --git a/windows/winpgen.c b/windows/winpgen.c index 065ada80..56c6a8db 100644 --- a/windows/winpgen.c +++ b/windows/winpgen.c @@ -1160,13 +1160,15 @@ static void start_generating_key(HWND hwnd, struct MainDlgState *state) params->key = &state->key; params->dsskey = &state->dsskey; - if (!CreateThread(NULL, 0, generate_key_thread, - params, 0, &threadid)) { + HANDLE hThread = CreateThread(NULL, 0, generate_key_thread, + params, 0, &threadid); + if (!hThread) { MessageBox(hwnd, "Out of thread resources", "Key generation error", MB_OK | MB_ICONERROR); sfree(params); } else { + CloseHandle(hThread); /* we don't need the thread handle */ state->generation_thread_exists = true; } } diff --git a/windows/winpgnt.c b/windows/winpgnt.c index c0ffcf63..d6e960b6 100644 --- a/windows/winpgnt.c +++ b/windows/winpgnt.c @@ -1656,8 +1656,10 @@ int WINAPI WinMain(HINSTANCE inst, HINSTANCE prev, LPSTR cmdline, int show) DWORD wm_copydata_threadid; wmct.ev_msg_ready = CreateEvent(NULL, false, false, NULL); wmct.ev_reply_ready = CreateEvent(NULL, false, false, NULL); - CreateThread(NULL, 0, wm_copydata_threadfunc, - &inst, 0, &wm_copydata_threadid); + HANDLE hThread = CreateThread(NULL, 0, wm_copydata_threadfunc, + &inst, 0, &wm_copydata_threadid); + if (hThread) + CloseHandle(hThread); /* we don't need the thread handle */ handle_add_foreign_event(wmct.ev_msg_ready, wm_copydata_got_msg, NULL); if (show_keylist_on_startup)