# [PATCH] win32: use _endthreadex to terminate threads, not ExitThread 5 messages from 2022-12-22 to 2022-12-25. Participants: Rose via GitGitGadget, Johannes Sixt, Junio C Hamano. Thread: https://gitlist.dev/t/58996 ## Rose via GitGitGadget, 2022-12-22 20:59 Subject: [PATCH] win32: use _endthreadex to terminate threads, not ExitThread Message-ID: URL: https://gitlist.dev/e/pull.1414.git.git.1671742750504.gitgitgadget%40gmail.com ``` From: Seija Kijin This is a pretty serious bug actually: Because we use the C runtime and use _beginthreadex to create pthreads, pthread_exit MUST use _endthreadex. Otherwise, according to Microsoft: "Failure to do so results in small memory leaks when the thread calls ExitThread." Simply put, this is not the same as ExitThread. Signed-off-by: Seija Kijin --- win32: use _endthreadex to terminate threads, not ExitThread This is a pretty serious bug actually: Because we use the C runtime and use _beginthread to create pthreads, pthread_exit MUST use _endthread. Otherwise, according to Microsoft: "Failure to do so results in small memory leaks when the thread calls ExitThread." Simply put, this is not the same as ExitThread. Signed-off-by: Seija Kijin doremylover123@gmail.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1414%2FAtariDreams%2Fsevere-bug-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1414/AtariDreams/severe-bug-v1 Pull-Request: https://github.com/git/git/pull/1414 compat/win32/pthread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 737983d00ba..cc3221cb2c8 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -66,7 +66,7 @@ pthread_t pthread_self(void); static inline void NORETURN pthread_exit(void *ret) { - ExitThread((DWORD)(intptr_t)ret); + _endthreadex((unsigned)(uintptr_t)ret); } typedef DWORD pthread_key_t; base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7 -- gitgitgadget ``` ## Johannes Sixt, 2022-12-24 08:01 Subject: Re: [PATCH] win32: use _endthreadex to terminate threads, not ExitThread Message-ID: <0b039029-97ce-bbc3-d9af-7f1eb1fd8b64@kdbg.org> URL: https://gitlist.dev/e/0b039029-97ce-bbc3-d9af-7f1eb1fd8b64%40kdbg.org In-Reply-To: ``` Am 22.12.22 um 21:59 schrieb Rose via GitGitGadget: > From: Seija Kijin > > This is a pretty serious bug actually: > Because we use the C runtime and > use _beginthreadex to create pthreads, > pthread_exit MUST use _endthreadex. > > Otherwise, according to Microsoft: > "Failure to do so results in small > memory leaks when the thread > calls ExitThread." > > Simply put, this is not the same as ExitThread. > > Signed-off-by: Seija Kijin > --- > compat/win32/pthread.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h > index 737983d00ba..cc3221cb2c8 100644 > --- a/compat/win32/pthread.h > +++ b/compat/win32/pthread.h > @@ -66,7 +66,7 @@ pthread_t pthread_self(void); > > static inline void NORETURN pthread_exit(void *ret) > { > - ExitThread((DWORD)(intptr_t)ret); > + _endthreadex((unsigned)(uintptr_t)ret); > } > > typedef DWORD pthread_key_t; Nice find! FWIW, this passes the test suite on Windows. The patch text is: Acked-by: Johannes Sixt The commit message is highly exaggerated, though. The bug is by no means serious. After all, we've been living with it for a decade. Notice that pthread_exit() is only called when a thread runs into die(). Even though we have a small memory leak, it does not occur in a loop because, after one thread dies, we do not tend to keep starting many, many more that all die. -- Hannes ``` ## Rose via GitGitGadget, 2022-12-25 01:41 Subject: [PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread Message-ID: URL: https://gitlist.dev/e/pull.1414.v2.git.git.1671932510529.gitgitgadget%40gmail.com In-Reply-To: ``` From: Seija Kijin Because we use the C runtime and use _beginthreadex to create pthreads, pthread_exit MUST use _endthreadex. Otherwise, according to Microsoft: "Failure to do so results in small memory leaks when the thread calls ExitThread." Simply put, this is not the same as ExitThread. Signed-off-by: Seija Kijin --- win32: use _endthreadex to terminate threads, not ExitThread Because we use the C runtime and use _beginthread to create pthreads, pthread_exit MUST use _endthread. Otherwise, according to Microsoft: "Failure to do so results in small memory leaks when the thread calls ExitThread." Simply put, this is not the same as ExitThread. Signed-off-by: Seija Kijin doremylover123@gmail.com Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1414%2FAtariDreams%2Fsevere-bug-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1414/AtariDreams/severe-bug-v2 Pull-Request: https://github.com/git/git/pull/1414 Range-diff vs v1: 1: 78f9d54c304 ! 1: 3e8212fb9a7 win32: use _endthreadex to terminate threads, not ExitThread @@ Metadata ## Commit message ## win32: use _endthreadex to terminate threads, not ExitThread - This is a pretty serious bug actually: Because we use the C runtime and use _beginthreadex to create pthreads, pthread_exit MUST use _endthreadex. compat/win32/pthread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 737983d00ba..cc3221cb2c8 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -66,7 +66,7 @@ pthread_t pthread_self(void); static inline void NORETURN pthread_exit(void *ret) { - ExitThread((DWORD)(intptr_t)ret); + _endthreadex((unsigned)(uintptr_t)ret); } typedef DWORD pthread_key_t; base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7 -- gitgitgadget ``` ## Johannes Sixt, 2022-12-25 08:54 Subject: Re: [PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread Message-ID: <983ce25f-82e1-e20d-ffb5-fd04fa9a9231@kdbg.org> URL: https://gitlist.dev/e/983ce25f-82e1-e20d-ffb5-fd04fa9a9231%40kdbg.org In-Reply-To: ``` Am 25.12.22 um 02:41 schrieb Rose via GitGitGadget: > From: Seija Kijin > > Because we use the C runtime and > use _beginthreadex to create pthreads, > pthread_exit MUST use _endthreadex. > > Otherwise, according to Microsoft: > "Failure to do so results in small > memory leaks when the thread > calls ExitThread." > > Simply put, this is not the same as ExitThread. > > Signed-off-by: Seija Kijin > --- > win32: use _endthreadex to terminate threads, not ExitThread > > Because we use the C runtime and use _beginthread to create pthreads, > pthread_exit MUST use _endthread. > > Otherwise, according to Microsoft: "Failure to do so results in small > memory leaks when the thread calls ExitThread." > > Simply put, this is not the same as ExitThread. > > Signed-off-by: Seija Kijin doremylover123@gmail.com > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1414%2FAtariDreams%2Fsevere-bug-v2 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1414/AtariDreams/severe-bug-v2 > Pull-Request: https://github.com/git/git/pull/1414 > > Range-diff vs v1: > > 1: 78f9d54c304 ! 1: 3e8212fb9a7 win32: use _endthreadex to terminate threads, not ExitThread > @@ Metadata > ## Commit message ## > win32: use _endthreadex to terminate threads, not ExitThread > > - This is a pretty serious bug actually: > Because we use the C runtime and > use _beginthreadex to create pthreads, > pthread_exit MUST use _endthreadex. > > > compat/win32/pthread.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h > index 737983d00ba..cc3221cb2c8 100644 > --- a/compat/win32/pthread.h > +++ b/compat/win32/pthread.h > @@ -66,7 +66,7 @@ pthread_t pthread_self(void); > > static inline void NORETURN pthread_exit(void *ret) > { > - ExitThread((DWORD)(intptr_t)ret); > + _endthreadex((unsigned)(uintptr_t)ret); > } > > typedef DWORD pthread_key_t; > > base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7 Thank you! This patch is now Acked-by: Johannes Sixt -- Hannes ``` ## Junio C Hamano, 2022-12-25 12:40 Subject: Re: [PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread Message-ID: URL: https://gitlist.dev/e/xmqqedsnodvb.fsf%40gitster.g In-Reply-To: <983ce25f-82e1-e20d-ffb5-fd04fa9a9231@kdbg.org> ``` Johannes Sixt writes: > Am 25.12.22 um 02:41 schrieb Rose via GitGitGadget: >> From: Seija Kijin >> ... > Thank you! This patch is now > > Acked-by: Johannes Sixt Thanks. Will queue. ```