win32: use _endthreadex to terminate threads, not ExitThread #1414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AtariDreams
force-pushed
the
severe-bug
branch
2 times, most recently
from
7153eb8
to
d65a60f
Compare
AtariDreams
changed the title
AtariDreams
force-pushed
the
severe-bug
branch
2 times, most recently
from
d1a1c1b
to
78f9d54
Compare
|
/submit |
|
Submitted as pull.1414.git.git.1671742750504.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 22.12.22 um 21:59 schrieb Rose via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
>
> 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 <doremylover123@gmail.com>
> ---
> 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 <j6t@kdbg.org>
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
|
|
User |
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 <doremylover123@gmail.com>
AtariDreams
force-pushed
the
severe-bug
branch
from
78f9d54
to
3e8212f
Compare
|
/submit |
|
Submitted as pull.1414.v2.git.git.1671932510529.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): Am 25.12.22 um 02:41 schrieb Rose via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
>
> 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 <doremylover123@gmail.com>
> ---
> 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 <j6t@kdbg.org>
-- Hannes
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Johannes Sixt <j6t@kdbg.org> writes:
> Am 25.12.22 um 02:41 schrieb Rose via GitGitGadget:
>> From: Seija Kijin <doremylover123@gmail.com>
>> ...
> Thank you! This patch is now
>
> Acked-by: Johannes Sixt <j6t@kdbg.org>
Thanks. Will queue. |
|
This patch series was integrated into seen via 7ea35b7. |
|
This branch is now known as |
|
This patch series was integrated into seen via b16751d. |
|
This patch series was integrated into next via ebcb1fe. |
|
There was a status update in the "New Topics" section about the branch An API emulation fix. Will merge to 'master'. source: <pull.1414.v2.git.git.1671932510529.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via 7d5e440. |
|
This patch series was integrated into seen via 039e5a0. |
|
This patch series was integrated into master via 039e5a0. |
|
This patch series was integrated into next via 039e5a0. |
|
Closed via 039e5a0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
cc: Johannes Sixt j6t@kdbg.org