threads / patch / 58996

patchwin32: use _endthreadex to terminate threads, not ExitThread

Subject: [PATCH] win32: use _endthreadex to terminate threads, not ExitThread

## tl;dr

5 messages between Dec 22, 2022 and Dec 25, 2022. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Rose via GitGitGadget· Dec 22, 2022, 20:59 UTC · lore
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>
---
    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(-)
Show changes to compat/win32/pthread.h +1 −1
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· Dec 24, 2022, 08:01 UTC · re: Rose via GitGitGadget · lore

Re: [PATCH] win32: use _endthreadex to terminate threads, not ExitThread

Am 22.12.22 um 21:59 schrieb Rose via GitGitGadget:
Show 16 quoted lines
> 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>
> ---
Show 16 quoted lines
>  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
Rose via GitGitGadget· Dec 25, 2022, 01:41 UTC · re: Rose via GitGitGadget · lore

[PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread

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(-)
Show changes to compat/win32/pthread.h +1 −1
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· Dec 25, 2022, 08:54 UTC · re: Rose via GitGitGadget · lore

Re: [PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread

Am 25.12.22 um 02:41 schrieb Rose via GitGitGadget:
Show 62 quoted lines
> 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
Junio C Hamano· Dec 25, 2022, 12:40 UTC · re: Johannes Sixt · lore

Re: [PATCH v2] win32: use _endthreadex to terminate threads, not ExitThread

Johannes Sixt <j6t@kdbg.org> writes:
Show 6 quoted lines
> 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.

← back to recent threads