[PATCH] win32: fix thread usage for win32

[PATCH] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use pthread_exit instead of async_exit.

This means we do not have
to deal with Windows's implementation
requiring an unsigned exit coded
despite the POSIX exit code requiring
a signed exit code.

Use _beginthreadex instead of CreateThread
since we use the Windows CRT.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v1
Pull-Request: https://github.com/git/git/pull/1440

 compat/mingw.c   |  2 +-
 compat/winansi.c |  8 ++++----
 run-command.c    | 33 ++++++++++++++-------------------
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index af397e68a1d..c41d821b382 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2295,7 +2295,7 @@ static int start_timer_thread(void)
 	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
 	if (timer_event) {
 		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
-		if (!timer_thread )
+		if (!timer_thread)
 			return errno = ENOMEM,
 				error("cannot start timer thread");
 	} else
diff --git a/compat/winansi.c b/compat/winansi.c
index 3abe8dd5a27..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
-	if (hthread == INVALID_HANDLE_VALUE)
-		die_lasterr("CreateThread(console_thread) failed");
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
+	if (!hthread)
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))
diff --git a/run-command.c b/run-command.c
index 50cc011654e..93fd0d22d4f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1030,6 +1030,13 @@ static void *run_thread(void *data)
 	return (void *)ret;
 }
 
+int in_async(void)
+{
+	if (!main_thread_set)
+		return 0; /* no asyncs started yet */
+	return !pthread_equal(main_thread, pthread_self());
+}
+
 static NORETURN void die_async(const char *err, va_list params)
 {
 	report_fn die_message_fn = get_die_message_routine();
@@ -1055,18 +1062,6 @@ static int async_die_is_recursing(void)
 	return ret != NULL;
 }
 
-int in_async(void)
-{
-	if (!main_thread_set)
-		return 0; /* no asyncs started yet */
-	return !pthread_equal(main_thread, pthread_self());
-}
-
-static void NORETURN async_exit(int code)
-{
-	pthread_exit((void *)(intptr_t)code);
-}
-
 #else
 
 static struct {
@@ -1112,18 +1107,18 @@ int in_async(void)
 	return process_is_async;
 }
 
-static void NORETURN async_exit(int code)
-{
-	exit(code);
-}
-
 #endif
 
 void check_pipe(int err)
 {
 	if (err == EPIPE) {
-		if (in_async())
-			async_exit(141);
+		if (in_async()) {
+#ifdef NO_PTHREADS
+			exit(141);
+#else
+			pthread_exit((void *)141);
+#endif
+		}
 
 		signal(SIGPIPE, SIG_DFL);
 		raise(SIGPIPE);

base-commit: 904d404274fef6695c78a6b055edd184b72e2f9b
-- 
gitgitgadget

Re: [PATCH] win32: fix thread usage for win32

[Johannes Sixt]

Am 21.01.23 um 21:49 schrieb Rose via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
> 
> Use pthread_exit instead of async_exit.
> 
> This means we do not have
> to deal with Windows's implementation
> requiring an unsigned exit coded
> despite the POSIX exit code requiring
> a signed exit code.
> 
> Use _beginthreadex instead of CreateThread
> since we use the Windows CRT.
> 
> Finally, check for NULL handles, not "INVALID_HANDLE,"
> as _beginthreadex guarantees a valid handle in most cases

This explains *what* the patch does, but not *why*. You replace
CreateThread() in winansi.c, but what has this to do with async_exit and
why must it be changed?

Please take the time to explain this story more thoroughly.

> 
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>     win32: fix thread usage for win32
>     
>     Use pthread_exit instead of async_exit.
>     
>     This means we do not have to deal with Windows's implementation
>     requiring an unsigned exit coded despite the POSIX exit code requiring a
>     signed exit code.
>     
>     Use _beginthreadex instead of CreateThread since we use the Windows CRT.
>     
>     Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
>     guarantees a valid handle in most cases
>     
>     Signed-off-by: Seija Kijin doremylover123@gmail.com
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v1
> Pull-Request: https://github.com/git/git/pull/1440
> 
>  compat/mingw.c   |  2 +-
>  compat/winansi.c |  8 ++++----
>  run-command.c    | 33 ++++++++++++++-------------------
>  3 files changed, 19 insertions(+), 24 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index af397e68a1d..c41d821b382 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2295,7 +2295,7 @@ static int start_timer_thread(void)
>  	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
>  	if (timer_event) {
>  		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
> -		if (!timer_thread )
> +		if (!timer_thread)
>  			return errno = ENOMEM,
>  				error("cannot start timer thread");
>  	} else
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3abe8dd5a27..be65b27bd75 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -340,7 +340,7 @@ enum {
>  	TEXT = 0, ESCAPE = 033, BRACKET = '['
>  };
>  
> -static DWORD WINAPI console_thread(LPVOID unused)
> +static unsigned int WINAPI console_thread(LPVOID unused)
>  {
>  	unsigned char buffer[BUFFER_SIZE];
>  	DWORD bytes;
> @@ -643,9 +643,9 @@ void winansi_init(void)
>  		die_lasterr("CreateFile for named pipe failed");
>  
>  	/* start console spool thread on the pipe's read end */
> -	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
> -	if (hthread == INVALID_HANDLE_VALUE)
> -		die_lasterr("CreateThread(console_thread) failed");
> +	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
> +	if (!hthread)
> +		die_lasterr("_beginthreadex(console_thread) failed");
>  
>  	/* schedule cleanup routine */
>  	if (atexit(winansi_exit))
> diff --git a/run-command.c b/run-command.c
> index 50cc011654e..93fd0d22d4f 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -1030,6 +1030,13 @@ static void *run_thread(void *data)
>  	return (void *)ret;
>  }
>  
> +int in_async(void)
> +{
> +	if (!main_thread_set)
> +		return 0; /* no asyncs started yet */
> +	return !pthread_equal(main_thread, pthread_self());
> +}
> +
>  static NORETURN void die_async(const char *err, va_list params)
>  {
>  	report_fn die_message_fn = get_die_message_routine();
> @@ -1055,18 +1062,6 @@ static int async_die_is_recursing(void)
>  	return ret != NULL;
>  }
>  
> -int in_async(void)
> -{
> -	if (!main_thread_set)
> -		return 0; /* no asyncs started yet */
> -	return !pthread_equal(main_thread, pthread_self());
> -}
> -
> -static void NORETURN async_exit(int code)
> -{
> -	pthread_exit((void *)(intptr_t)code);
> -}
> -
>  #else
>  
>  static struct {
> @@ -1112,18 +1107,18 @@ int in_async(void)
>  	return process_is_async;
>  }
>  
> -static void NORETURN async_exit(int code)
> -{
> -	exit(code);
> -}
> -
>  #endif
>  
>  void check_pipe(int err)
>  {
>  	if (err == EPIPE) {
> -		if (in_async())
> -			async_exit(141);
> +		if (in_async()) {
> +#ifdef NO_PTHREADS
> +			exit(141);
> +#else
> +			pthread_exit((void *)141);
> +#endif
> +		}
>  
>  		signal(SIGPIPE, SIG_DFL);
>  		raise(SIGPIPE);
> 
> base-commit: 904d404274fef6695c78a6b055edd184b72e2f9b

[PATCH v2] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v2
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v1:

 1:  f5de6bfb759 ! 1:  4a2c3da9d4c win32: fix thread usage for win32
     @@ Metadata
       ## Commit message ##
          win32: fix thread usage for win32
      
     -    Use pthread_exit instead of async_exit.
     -
     -    This means we do not have
     -    to deal with Windows's implementation
     -    requiring an unsigned exit coded
     -    despite the POSIX exit code requiring
     -    a signed exit code.
     -
          Use _beginthreadex instead of CreateThread
          since we use the Windows CRT.
      
     @@ compat/winansi.c: void winansi_init(void)
       
       	/* schedule cleanup routine */
       	if (atexit(winansi_exit))
     -
     - ## run-command.c ##
     -@@ run-command.c: static void *run_thread(void *data)
     - 	return (void *)ret;
     - }
     - 
     -+int in_async(void)
     -+{
     -+	if (!main_thread_set)
     -+		return 0; /* no asyncs started yet */
     -+	return !pthread_equal(main_thread, pthread_self());
     -+}
     -+
     - static NORETURN void die_async(const char *err, va_list params)
     - {
     - 	report_fn die_message_fn = get_die_message_routine();
     -@@ run-command.c: static int async_die_is_recursing(void)
     - 	return ret != NULL;
     - }
     - 
     --int in_async(void)
     --{
     --	if (!main_thread_set)
     --		return 0; /* no asyncs started yet */
     --	return !pthread_equal(main_thread, pthread_self());
     --}
     --
     --static void NORETURN async_exit(int code)
     --{
     --	pthread_exit((void *)(intptr_t)code);
     --}
     --
     - #else
     - 
     - static struct {
     -@@ run-command.c: int in_async(void)
     - 	return process_is_async;
     - }
     - 
     --static void NORETURN async_exit(int code)
     --{
     --	exit(code);
     --}
     --
     - #endif
     - 
     - void check_pipe(int err)
     - {
     - 	if (err == EPIPE) {
     --		if (in_async())
     --			async_exit(141);
     -+		if (in_async()) {
     -+#ifdef NO_PTHREADS
     -+			exit(141);
     -+#else
     -+			pthread_exit((void *)141);
     -+#endif
     -+		}
     - 
     - 		signal(SIGPIPE, SIG_DFL);
     - 		raise(SIGPIPE);


 compat/mingw.c   | 2 +-
 compat/winansi.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index e433740381b..715f1c87e11 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2291,7 +2291,7 @@ static int start_timer_thread(void)
 	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
 	if (timer_event) {
 		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
-		if (!timer_thread )
+		if (!timer_thread)
 			return errno = ENOMEM,
 				error("cannot start timer thread");
 	} else
diff --git a/compat/winansi.c b/compat/winansi.c
index 3abe8dd5a27..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
-	if (hthread == INVALID_HANDLE_VALUE)
-		die_lasterr("CreateThread(console_thread) failed");
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
+	if (!hthread)
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2
-- 
gitgitgadget

[PATCH v3] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT,
as Microsoft recommends _beginthreadex
over CreateThread for these situations.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v3
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v2:

 1:  4a2c3da9d4c ! 1:  68baafba2bd win32: fix thread usage for win32
     @@ Commit message
          win32: fix thread usage for win32
      
          Use _beginthreadex instead of CreateThread
     -    since we use the Windows CRT.
     +    since we use the Windows CRT,
     +    as Microsoft recommends _beginthreadex
     +    over CreateThread for these situations.
      
          Finally, check for NULL handles, not "INVALID_HANDLE,"
          as _beginthreadex guarantees a valid handle in most cases


 compat/mingw.c   | 2 +-
 compat/winansi.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index e433740381b..715f1c87e11 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2291,7 +2291,7 @@ static int start_timer_thread(void)
 	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
 	if (timer_event) {
 		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
-		if (!timer_thread )
+		if (!timer_thread)
 			return errno = ENOMEM,
 				error("cannot start timer thread");
 	} else
diff --git a/compat/winansi.c b/compat/winansi.c
index 3abe8dd5a27..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
-	if (hthread == INVALID_HANDLE_VALUE)
-		die_lasterr("CreateThread(console_thread) failed");
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
+	if (!hthread)
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2
-- 
gitgitgadget

[PATCH v4] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT,
as Microsoft recommends _beginthreadex
over CreateThread for these situations.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v4
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v3:

 1:  68baafba2bd ! 1:  2e2d5ce7745 win32: fix thread usage for win32
     @@ Commit message
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
     - ## compat/mingw.c ##
     -@@ compat/mingw.c: static int start_timer_thread(void)
     - 	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
     - 	if (timer_event) {
     - 		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
     --		if (!timer_thread )
     -+		if (!timer_thread)
     - 			return errno = ENOMEM,
     - 				error("cannot start timer thread");
     - 	} else
     -
       ## compat/winansi.c ##
      @@ compat/winansi.c: enum {
       	TEXT = 0, ESCAPE = 033, BRACKET = '['


 compat/winansi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index 3abe8dd5a27..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
-	if (hthread == INVALID_HANDLE_VALUE)
-		die_lasterr("CreateThread(console_thread) failed");
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
+	if (!hthread)
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2
-- 
gitgitgadget

Re: [PATCH v4] win32: fix thread usage for win32

[Jeff Hostetler]

On 1/23/23 11:48 AM, Rose via GitGitGadget wrote:
> From: Seija Kijin <doremylover123@gmail.com>
> 
> Use _beginthreadex instead of CreateThread
> since we use the Windows CRT,
> as Microsoft recommends _beginthreadex
> over CreateThread for these situations.
> 
> Finally, check for NULL handles, not "INVALID_HANDLE,"
> as _beginthreadex guarantees a valid handle in most cases
> 
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>      win32: fix thread usage for win32
>      
>      Use pthread_exit instead of async_exit.
>      
>      This means we do not have to deal with Windows's implementation
>      requiring an unsigned exit coded despite the POSIX exit code requiring a
>      signed exit code.
>      
>      Use _beginthreadex instead of CreateThread since we use the Windows CRT.
>      
>      Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
>      guarantees a valid handle in most cases
>      
>      Signed-off-by: Seija Kijin doremylover123@gmail.com
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v4
> Pull-Request: https://github.com/git/git/pull/1440
> 
> Range-diff vs v3:
> 
>   1:  68baafba2bd ! 1:  2e2d5ce7745 win32: fix thread usage for win32
>       @@ Commit message
>        
>            Signed-off-by: Seija Kijin <doremylover123@gmail.com>
>        
>       - ## compat/mingw.c ##
>       -@@ compat/mingw.c: static int start_timer_thread(void)
>       - 	timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
>       - 	if (timer_event) {
>       - 		timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
>       --		if (!timer_thread )
>       -+		if (!timer_thread)
>       - 			return errno = ENOMEM,
>       - 				error("cannot start timer thread");
>       - 	} else
>       -
>         ## compat/winansi.c ##
>        @@ compat/winansi.c: enum {
>         	TEXT = 0, ESCAPE = 033, BRACKET = '['
> 
> 
>   compat/winansi.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 3abe8dd5a27..be65b27bd75 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -340,7 +340,7 @@ enum {
>   	TEXT = 0, ESCAPE = 033, BRACKET = '['
>   };
>   
> -static DWORD WINAPI console_thread(LPVOID unused)
> +static unsigned int WINAPI console_thread(LPVOID unused)
>   {
>   	unsigned char buffer[BUFFER_SIZE];
>   	DWORD bytes;
> @@ -643,9 +643,9 @@ void winansi_init(void)
>   		die_lasterr("CreateFile for named pipe failed");
>   
>   	/* start console spool thread on the pipe's read end */
> -	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
> -	if (hthread == INVALID_HANDLE_VALUE)
> -		die_lasterr("CreateThread(console_thread) failed");
> +	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
> +	if (!hthread)
> +		die_lasterr("_beginthreadex(console_thread) failed");
>   
>   	/* schedule cleanup routine */
>   	if (atexit(winansi_exit))
> 
> base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2

This change may or may not be harmless, but it scares me
because it is possibly a very subtle change and is being
made for an unknown reason -- is there a problem being
fixed here?  Or is this just churn for the sake of churn
to avoid an awkward cast of the return code?

What does _beginthreadex() specifically do that we need
it to do for us?

_beginthreadex() does some CRT init and then calls CreateThread(),
so what are we missing by calling CreateThread() directly?

The code in question is 11+ years old and it hasn't been a
problem (right?), so I have to wonder what value do we get
from this change.

The containing function here is setting up a special console
thread and named pipe to access the console, so I doubt that
any of the tests in the test suite actually would actually
exercise this change (since the tests aren't interactive).

The low-level Windows startup code is very tricky and sensitive
(and we need to test with both GCC's CRT and MSVC's CRT).
As I said earlier, the change may or may not be harmless, but
I question the need for it.

Jeff

Re: [PATCH v4] win32: fix thread usage for win32

[Johannes Sixt]

Am 23.01.23 um 18:43 schrieb Jeff Hostetler:
> 
> 
> On 1/23/23 11:48 AM, Rose via GitGitGadget wrote:
>> From: Seija Kijin <doremylover123@gmail.com>
>>
>> Use _beginthreadex instead of CreateThread
>> since we use the Windows CRT,
>> as Microsoft recommends _beginthreadex
>> over CreateThread for these situations.
>>
>> Finally, check for NULL handles, not "INVALID_HANDLE,"
>> as _beginthreadex guarantees a valid handle in most cases
>>
>> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
>> ---
>>      win32: fix thread usage for win32
>>           Use pthread_exit instead of async_exit.
>>           This means we do not have to deal with Windows's implementation
>>      requiring an unsigned exit coded despite the POSIX exit code
>> requiring a
>>      signed exit code.
>>           Use _beginthreadex instead of CreateThread since we use the
>> Windows CRT.
>>           Finally, check for NULL handles, not "INVALID_HANDLE," as
>> _beginthreadex
>>      guarantees a valid handle in most cases
>>           Signed-off-by: Seija Kijin doremylover123@gmail.com
>>
>> Published-As:
>> https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v4
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git
>> pr-git-1440/AtariDreams/CreateThread-v4
>> Pull-Request: https://github.com/git/git/pull/1440
>>
>> Range-diff vs v3:
>>
>>   1:  68baafba2bd ! 1:  2e2d5ce7745 win32: fix thread usage for win32
>>       @@ Commit message
>>                   Signed-off-by: Seija Kijin <doremylover123@gmail.com>
>>              - ## compat/mingw.c ##
>>       -@@ compat/mingw.c: static int start_timer_thread(void)
>>       -     timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
>>       -     if (timer_event) {
>>       -         timer_thread = (HANDLE) _beginthreadex(NULL, 0,
>> ticktack, NULL, 0, NULL);
>>       --        if (!timer_thread )
>>       -+        if (!timer_thread)
>>       -             return errno = ENOMEM,
>>       -                 error("cannot start timer thread");
>>       -     } else
>>       -
>>         ## compat/winansi.c ##
>>        @@ compat/winansi.c: enum {
>>             TEXT = 0, ESCAPE = 033, BRACKET = '['
>>
>>
>>   compat/winansi.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/compat/winansi.c b/compat/winansi.c
>> index 3abe8dd5a27..be65b27bd75 100644
>> --- a/compat/winansi.c
>> +++ b/compat/winansi.c
>> @@ -340,7 +340,7 @@ enum {
>>       TEXT = 0, ESCAPE = 033, BRACKET = '['
>>   };
>>   -static DWORD WINAPI console_thread(LPVOID unused)
>> +static unsigned int WINAPI console_thread(LPVOID unused)
>>   {
>>       unsigned char buffer[BUFFER_SIZE];
>>       DWORD bytes;
>> @@ -643,9 +643,9 @@ void winansi_init(void)
>>           die_lasterr("CreateFile for named pipe failed");
>>         /* start console spool thread on the pipe's read end */
>> -    hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
>> -    if (hthread == INVALID_HANDLE_VALUE)
>> -        die_lasterr("CreateThread(console_thread) failed");
>> +    hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL,
>> 0, NULL);
>> +    if (!hthread)
>> +        die_lasterr("_beginthreadex(console_thread) failed");
>>         /* schedule cleanup routine */
>>       if (atexit(winansi_exit))
>>
>> base-commit: 56c8fb1e95377900ec9d53c07886022af0a5d3c2
> 
> This change may or may not be harmless, but it scares me
> because it is possibly a very subtle change and is being
> made for an unknown reason -- is there a problem being
> fixed here?  Or is this just churn for the sake of churn
> to avoid an awkward cast of the return code?
> 
> What does _beginthreadex() specifically do that we need
> it to do for us?
> 
> _beginthreadex() does some CRT init and then calls CreateThread(),
> so what are we missing by calling CreateThread() directly?

I also question the value of this change. As long as the thread does not
call into any CRT functions, we do not need the services of
_beginthreadex(). AFAICS, it only uses WinAPI functions and some
uncritical C functions like memmove and memset. Am I missing something?

> 
> The code in question is 11+ years old and it hasn't been a
> problem (right?), so I have to wonder what value do we get
> from this change.
> 
> The containing function here is setting up a special console
> thread and named pipe to access the console, so I doubt that
> any of the tests in the test suite actually would actually
> exercise this change (since the tests aren't interactive).
> 
> The low-level Windows startup code is very tricky and sensitive
> (and we need to test with both GCC's CRT and MSVC's CRT).
> As I said earlier, the change may or may not be harmless, but
> I question the need for it.
> 
> Jeff
> 
> 

[PATCH v5] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT,
as Microsoft recommends _beginthreadex
over CreateThread for these situations.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v5
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v4:

 1:  2e2d5ce7745 = 1:  6ab79d9275d win32: fix thread usage for win32


 compat/winansi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index 3abe8dd5a27..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
-	if (hthread == INVALID_HANDLE_VALUE)
-		die_lasterr("CreateThread(console_thread) failed");
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
+	if (!hthread)
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 2fc9e9ca3c7505bc60069f11e7ef09b1aeeee473
-- 
gitgitgadget

Re: [PATCH v5] win32: fix thread usage for win32

[Johannes Sixt]

Am 31.01.23 um 15:47 schrieb Rose via GitGitGadget:
> Range-diff vs v4:
> 
>  1:  2e2d5ce7745 = 1:  6ab79d9275d win32: fix thread usage for win32

If you do not agree with review comments, it would be more polite to
express your thinking than to just resend a patch unchanged and silently.

-- Hannes

[PATCH v6] win32: fix thread usage for win32

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT,
as Microsoft recommends _beginthreadex
over CreateThread for these situations.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: fix thread usage for win32
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v6
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v5:

 1:  6ab79d9275d ! 1:  67cef4bd8c9 win32: fix thread usage for win32
     @@ compat/winansi.c: void winansi_init(void)
       
       	/* start console spool thread on the pipe's read end */
      -	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
     --	if (hthread == INVALID_HANDLE_VALUE)
     --		die_lasterr("CreateThread(console_thread) failed");
      +	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
     -+	if (!hthread)
     + 	if (!hthread)
     +-		die_lasterr("CreateThread(console_thread) failed");
      +		die_lasterr("_beginthreadex(console_thread) failed");
       
       	/* schedule cleanup routine */


 compat/winansi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index f83610f684d..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
 	if (!hthread)
-		die_lasterr("CreateThread(console_thread) failed");
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6
-- 
gitgitgadget

[PATCH v7] win32: prefer beginthreadex over CreateThread

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Use _beginthreadex instead of CreateThread
since we use the Windows CRT,
as Microsoft recommends _beginthreadex
over CreateThread for these situations.

Finally, check for NULL handles, not "INVALID_HANDLE,"
as _beginthreadex guarantees a valid handle in most cases

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: prefer beginthreadex over CreateThread
    
    Use pthread_exit instead of async_exit.
    
    This means we do not have to deal with Windows's implementation
    requiring an unsigned exit coded despite the POSIX exit code requiring a
    signed exit code.
    
    Use _beginthreadex instead of CreateThread since we use the Windows CRT.
    
    Finally, check for NULL handles, not "INVALID_HANDLE," as _beginthreadex
    guarantees a valid handle in most cases
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1440%2FAtariDreams%2FCreateThread-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1440/AtariDreams/CreateThread-v7
Pull-Request: https://github.com/git/git/pull/1440

Range-diff vs v6:

 1:  67cef4bd8c9 ! 1:  edee5e78c67 win32: fix thread usage for win32
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32: fix thread usage for win32
     +    win32: prefer beginthreadex over CreateThread
      
          Use _beginthreadex instead of CreateThread
          since we use the Windows CRT,


 compat/winansi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compat/winansi.c b/compat/winansi.c
index f83610f684d..be65b27bd75 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -340,7 +340,7 @@ enum {
 	TEXT = 0, ESCAPE = 033, BRACKET = '['
 };
 
-static DWORD WINAPI console_thread(LPVOID unused)
+static unsigned int WINAPI console_thread(LPVOID unused)
 {
 	unsigned char buffer[BUFFER_SIZE];
 	DWORD bytes;
@@ -643,9 +643,9 @@ void winansi_init(void)
 		die_lasterr("CreateFile for named pipe failed");
 
 	/* start console spool thread on the pipe's read end */
-	hthread = CreateThread(NULL, 0, console_thread, NULL, 0, NULL);
+	hthread = (HANDLE)_beginthreadex(NULL, 0, console_thread, NULL, 0, NULL);
 	if (!hthread)
-		die_lasterr("CreateThread(console_thread) failed");
+		die_lasterr("_beginthreadex(console_thread) failed");
 
 	/* schedule cleanup routine */
 	if (atexit(winansi_exit))

base-commit: 23c56f7bd5f1667f8b793d796bf30e39545920f6
-- 
gitgitgadget