[PATCH] win32: close handles of threads that have been joined

[PATCH] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: close handles of threads that have been joined
    
    After joining threads, the handle to the original thread should be
    closed as it no longer needs to be open.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

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

 compat/win32/pthread.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..de89667ef70 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -39,14 +39,20 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		/* detach the thread once the join succeeds */
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		/* either thread is not joinable or another thread is waiting on
+		 * this, so we do not detatch */
+		return EINVAL;
+	default:
+	case WAIT_FAILED:
+		/* the function failed so we do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 

base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
-- 
gitgitgadget

Re: [PATCH] win32: close handles of threads that have been joined

[Ævar Arnfjörð Bjarmason]

On Mon, Dec 19 2022, Rose via GitGitGadget wrote:

> From: Seija Kijin <doremylover123@gmail.com>
>
> After joining threads, the handle to the original thread
> should be closed as it no longer needs to be open.
>
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>     win32: close handles of threads that have been joined
>     
>     After joining threads, the handle to the original thread should be
>     closed as it no longer needs to be open.
>     
>     Signed-off-by: Seija Kijin doremylover123@gmail.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v1
> Pull-Request: https://github.com/git/git/pull/1406
>
>  compat/win32/pthread.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 2e7eead42cb..de89667ef70 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -39,14 +39,20 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  {
>  	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
>  	switch (result) {
> -		case WAIT_OBJECT_0:
> -			if (value_ptr)
> -				*value_ptr = thread->arg;
> -			return 0;
> -		case WAIT_ABANDONED:
> -			return EINVAL;
> -		default:
> -			return err_win_to_posix(GetLastError());
> +	case WAIT_OBJECT_0:
> +		if (value_ptr)
> +			*value_ptr = thread->arg;
> +		/* detach the thread once the join succeeds */
> +		CloseHandle(thread->handle);
> +		return 0;
> +	case WAIT_ABANDONED:
> +		/* either thread is not joinable or another thread is waiting on
> +		 * this, so we do not detatch */

See CodingGuidelines for how multi-line comments should look like.

	/*
	 * Like this
	 * Another line etc.
	 */
> +		return EINVAL;
> +	default:
> +	case WAIT_FAILED:
> +		/* the function failed so we do not detach */
> +		return err_win_to_posix(GetLastError());

The post-image adhares to our CodingGuidelines better than the
pre-image, but please split up such re-indentation into a "prep" change.
Manually looking at this with "git show -w" shows the actual (and
smaller) functional change.

You add a "case" for WAIT_FAILED", but keep "default".

I have no idea about this API, but a search turned up:
https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject

That seems to suggest that it only returns 4 possible values.

Rather than having the "default" case shouldn't we (and this is just a
suggestion, and should be its own prep change in any case) do:

	switch (result) {
	case WAIT_OBJECT_0:
		return ...;
	case WAIT_ABANDONED:
		return ...;
	case WAIT_TIMEOUT:
	case WAIT_FAILED:
		return ...;
	default:
		BUG("unhandled result %d", result);
	}

I.e. instead of keeping "default" you can just list "WAIT_TIMEOUT".

I don't know if that's OK with this API, it does say "If the function
succeeds, the return value indicates, so maybe that "default" handles a
lot more still?

Re: [PATCH] win32: close handles of threads that have been joined

[Johannes Sixt]

Am 19.12.22 um 19:34 schrieb Rose via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
> 
> After joining threads, the handle to the original thread
> should be closed as it no longer needs to be open.
> 
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---

>  compat/win32/pthread.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 2e7eead42cb..de89667ef70 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -39,14 +39,20 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  {
>  	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
>  	switch (result) {
> -		case WAIT_OBJECT_0:
> -			if (value_ptr)
> -				*value_ptr = thread->arg;
> -			return 0;
> -		case WAIT_ABANDONED:
> -			return EINVAL;
> -		default:
> -			return err_win_to_posix(GetLastError());
> +	case WAIT_OBJECT_0:
> +		if (value_ptr)
> +			*value_ptr = thread->arg;
> +		/* detach the thread once the join succeeds */
> +		CloseHandle(thread->handle);
> +		return 0;

This is a good change. It is a severe omission that the handle was not
closed. (But I still have to test the patch.)

> +	case WAIT_ABANDONED:
> +		/* either thread is not joinable or another thread is waiting on
> +		 * this, so we do not detatch */
> +		return EINVAL;

I don't know which cases this mental note wants to help. Assuming that
the [win232_]pthread_ API is used correctly, this error cannot happen
(WAIT_ABANDONED can only happen when WaitForSingleObject is called on a
mutex object).

> +	default:
> +	case WAIT_FAILED:
> +		/* the function failed so we do not detach */
> +		return err_win_to_posix(GetLastError());
>  	}

Good.

-- Hannes

[PATCH v2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: close handles of threads that have been joined
    
    After joining threads, the handle to the original thread should be
    closed as it no longer needs to be open.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

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

Range-diff vs v1:

 1:  e53b274ab4b ! 1:  8b20c4149be win32: close handles of threads that have been joined
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +		return 0;
      +	case WAIT_ABANDONED:
      +		/* either thread is not joinable or another thread is waiting on
     -+		 * this, so we do not detatch */
     ++		 * this, so do not detatch */
      +		return EINVAL;
     -+	default:
      +	case WAIT_FAILED:
     ++	default:
      +		/* the function failed so we do not detach */
      +		return err_win_to_posix(GetLastError());
       	}


 compat/win32/pthread.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..0d62c45ecc2 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -39,14 +39,20 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		/* detach the thread once the join succeeds */
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		/* either thread is not joinable or another thread is waiting on
+		 * this, so do not detatch */
+		return EINVAL;
+	case WAIT_FAILED:
+	default:
+		/* the function failed so we do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 

base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
-- 
gitgitgadget

[PATCH v3] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: close handles of threads that have been joined
    
    After joining threads, the handle to the original thread should be
    closed as it no longer needs to be open.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

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

Range-diff vs v2:

 1:  8b20c4149be ! 1:  2049576b193 win32: close handles of threads that have been joined
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +		return EINVAL;
      +	case WAIT_FAILED:
      +	default:
     -+		/* the function failed so we do not detach */
     ++		/* the function failed, so do not detach */
      +		return err_win_to_posix(GetLastError());
       	}
       }


 compat/win32/pthread.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..306834a7153 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -39,14 +39,20 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		/* detach the thread once the join succeeds */
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		/* either thread is not joinable or another thread is waiting on
+		 * this, so do not detatch */
+		return EINVAL;
+	case WAIT_FAILED:
+	default:
+		/* the function failed, so do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 

base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
-- 
gitgitgadget

[PATCH v4 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: close handles of threads that have been joined
  prep

 compat/win32/pthread.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v4
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v3:

 -:  ----------- > 1:  526ef7cc339 win32: close handles of threads that have been joined
 1:  2049576b193 ! 2:  2cb4d5c7007 win32: close handles of threads that have been joined
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32: close handles of threads that have been joined
     -
     -    After joining threads, the handle to the original thread
     -    should be closed as it no longer needs to be open.
     +    prep
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
       ## compat/win32/pthread.c ##
     +@@ compat/win32/pthread.c: static unsigned __stdcall win32_start_routine(void *arg)
     + }
     + 
     + int pthread_create(pthread_t *thread, const void *unused,
     +-		   void *(*start_routine)(void*), void *arg)
     ++		   void *(*start_routine)(void *), void *arg)
     + {
     + 	thread->arg = arg;
     + 	thread->start_routine = start_routine;
     +-	thread->handle = (HANDLE)
     +-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
     ++	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
     ++						thread, 0, NULL);
     + 
     + 	if (!thread->handle)
     + 		return errno;
      @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_ptr)
       {
       	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      -		case WAIT_OBJECT_0:
      -			if (value_ptr)
      -				*value_ptr = thread->arg;
     +-			/* detach the thread once the join succeeds */
     +-			CloseHandle(thread->handle);
      -			return 0;
      -		case WAIT_ABANDONED:
     +-			/* either thread is not joinable or another thread is
     +-			 * waiting on this, so do not detatch */
      -			return EINVAL;
      -		default:
     +-			/* the function failed, so do not detach */
      -			return err_win_to_posix(GetLastError());
      +	case WAIT_OBJECT_0:
      +		if (value_ptr)
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +		CloseHandle(thread->handle);
      +		return 0;
      +	case WAIT_ABANDONED:
     -+		/* either thread is not joinable or another thread is waiting on
     -+		 * this, so do not detatch */
     ++		/* either thread is not joinable or another thread is
     ++		 * waiting on this, so do not detatch */
      +		return EINVAL;
     -+	case WAIT_FAILED:
      +	default:
      +		/* the function failed, so do not detach */
      +		return err_win_to_posix(GetLastError());

-- 
gitgitgadget

[PATCH v4 1/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..89e89c3fe00 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,15 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 		case WAIT_OBJECT_0:
 			if (value_ptr)
 				*value_ptr = thread->arg;
+			/* detach the thread once the join succeeds */
+			CloseHandle(thread->handle);
 			return 0;
 		case WAIT_ABANDONED:
+			/* either thread is not joinable or another thread is
+			 * waiting on this, so do not detatch */
 			return EINVAL;
 		default:
+			/* the function failed, so do not detach */
 			return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

[PATCH v4 2/2] prep

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 89e89c3fe00..0831ffbc3ae 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -39,19 +39,19 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			/* detach the thread once the join succeeds */
-			CloseHandle(thread->handle);
-			return 0;
-		case WAIT_ABANDONED:
-			/* either thread is not joinable or another thread is
-			 * waiting on this, so do not detatch */
-			return EINVAL;
-		default:
-			/* the function failed, so do not detach */
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		/* detach the thread once the join succeeds */
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		/* either thread is not joinable or another thread is
+		 * waiting on this, so do not detatch */
+		return EINVAL;
+	default:
+		/* the function failed, so do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v5] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

Because this only needs to happen if the
WaitForSingleObject fails, the function was
rewritten to accommodate this change.

The function is still POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: close handles of threads that have been joined
    
    After joining threads, the handle to the original thread should be
    closed as it no longer needs to be open.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

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

Range-diff vs v4:

 1:  526ef7cc339 < -:  ----------- win32: close handles of threads that have been joined
 2:  2cb4d5c7007 ! 1:  94ed068d25b prep
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    prep
     +    win32: close handles of threads that have been joined
     +
     +    After joining threads, the handle to the original thread
     +    should be closed as it no longer needs to be open.
     +
     +    Because this only needs to happen if the
     +    WaitForSingleObject fails, the function was
     +    rewritten to accommodate this change.
     +
     +    The function is still POSIX compliant.
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
       ## compat/win32/pthread.c ##
     -@@ compat/win32/pthread.c: static unsigned __stdcall win32_start_routine(void *arg)
     - }
     - 
     - int pthread_create(pthread_t *thread, const void *unused,
     --		   void *(*start_routine)(void*), void *arg)
     -+		   void *(*start_routine)(void *), void *arg)
     - {
     - 	thread->arg = arg;
     - 	thread->start_routine = start_routine;
     --	thread->handle = (HANDLE)
     --		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
     -+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
     -+						thread, 0, NULL);
     +@@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused,
       
     - 	if (!thread->handle)
     - 		return errno;
     -@@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_ptr)
     + int win32_pthread_join(pthread_t *thread, void **value_ptr)
       {
     - 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
     - 	switch (result) {
     +-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
     +-	switch (result) {
      -		case WAIT_OBJECT_0:
      -			if (value_ptr)
      -				*value_ptr = thread->arg;
     --			/* detach the thread once the join succeeds */
     --			CloseHandle(thread->handle);
      -			return 0;
      -		case WAIT_ABANDONED:
     --			/* either thread is not joinable or another thread is
     --			 * waiting on this, so do not detatch */
      -			return EINVAL;
      -		default:
     --			/* the function failed, so do not detach */
      -			return err_win_to_posix(GetLastError());
     -+	case WAIT_OBJECT_0:
     -+		if (value_ptr)
     -+			*value_ptr = thread->arg;
     -+		/* detach the thread once the join succeeds */
     -+		CloseHandle(thread->handle);
     -+		return 0;
     -+	case WAIT_ABANDONED:
     -+		/* either thread is not joinable or another thread is
     -+		 * waiting on this, so do not detatch */
     -+		return EINVAL;
     -+	default:
     -+		/* the function failed, so do not detach */
     ++	if (WaitForSingleObject(thread->handle, INFINITE) == WAIT_FAILED)
      +		return err_win_to_posix(GetLastError());
     ++
     ++	if (value_ptr) {
     ++		*value_ptr = thread->arg;
       	}
     ++
     ++	CloseHandle(thread->handle);
     ++	return 0;
       }
       
     + pthread_t pthread_self(void)


 compat/win32/pthread.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..7f8503b4b50 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -37,17 +37,15 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	if (WaitForSingleObject(thread->handle, INFINITE) == WAIT_FAILED)
+		return err_win_to_posix(GetLastError());
+
+	if (value_ptr) {
+		*value_ptr = thread->arg;
 	}
+
+	CloseHandle(thread->handle);
+	return 0;
 }
 
 pthread_t pthread_self(void)

base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
-- 
gitgitgadget

Re: [PATCH v5] win32: close handles of threads that have been joined

[Johannes Sixt]

Am 20.12.22 um 22:18 schrieb Rose via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
> 
> After joining threads, the handle to the original thread
> should be closed as it no longer needs to be open.
> 
> Because this only needs to happen if the
> WaitForSingleObject fails, the function was
> rewritten to accommodate this change.

This sentence says that the handle must be closed only when
WaitForSingleObject fails. But my understanding is that we must close it
when the call is successful. In fact, that is what you implemented.

> The function is still POSIX compliant.
> 
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---

> 
>  compat/win32/pthread.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 2e7eead42cb..7f8503b4b50 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -37,17 +37,15 @@ int pthread_create(pthread_t *thread, const void *unused,
>  
>  int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  {
> -	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
> -	switch (result) {
> -		case WAIT_OBJECT_0:
> -			if (value_ptr)
> -				*value_ptr = thread->arg;
> -			return 0;
> -		case WAIT_ABANDONED:
> -			return EINVAL;
> -		default:
> -			return err_win_to_posix(GetLastError());
> +	if (WaitForSingleObject(thread->handle, INFINITE) == WAIT_FAILED)
> +		return err_win_to_posix(GetLastError());
> +
> +	if (value_ptr) {
> +		*value_ptr = thread->arg;
>  	}
> +
> +	CloseHandle(thread->handle);
> +	return 0;

Generally, such rewrites are not welcome if there is no obvious value in
the new code structure. To my eyes, the original switch statement is
much clearer than the new structure. In particular, the good case is
when the result is WAIT_OBJECT_0. The switch statement clearly handles
the case. The new code, however, loses the handling of a buggy caller,
WAIT_ABANONED, and handles it like a success case.

What is wrong with just inserting a CloseHandle() call at the right spot
in the original code and no other change?

>  }
>  
>  pthread_t pthread_self(void)


-- Hannes

[PATCH v6 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: close handles of threads that have been joined
  prep

 compat/win32/pthread.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v6
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v5:

 1:  94ed068d25b ! 1:  0b5afdb93db win32: close handles of threads that have been joined
     @@ Commit message
          After joining threads, the handle to the original thread
          should be closed as it no longer needs to be open.
      
     -    Because this only needs to happen if the
     -    WaitForSingleObject fails, the function was
     -    rewritten to accommodate this change.
     -
     -    The function is still POSIX compliant.
     +    This change makes the function POSIX compliant.
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
     @@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused
      -			return EINVAL;
      -		default:
      -			return err_win_to_posix(GetLastError());
     -+	if (WaitForSingleObject(thread->handle, INFINITE) == WAIT_FAILED)
     ++	switch (WaitForSingleObject(thread->handle, INFINITE)) {
     ++	case WAIT_OBJECT_0:
     ++		if (value_ptr)
     ++			*value_ptr = thread->arg;
     ++		CloseHandle(thread->handle);
     ++		return 0;
     ++	case WAIT_ABANDONED:
     ++		CloseHandle(thread->handle);
     ++		return EINVAL;
     ++	default:
     ++		/* the function failed, so do not detach */
      +		return err_win_to_posix(GetLastError());
     -+
     -+	if (value_ptr) {
     -+		*value_ptr = thread->arg;
       	}
     -+
     -+	CloseHandle(thread->handle);
     -+	return 0;
       }
       
     - pthread_t pthread_self(void)
 -:  ----------- > 2:  5b35362e5d2 prep

-- 
gitgitgadget

[PATCH v6 1/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After joining threads, the handle to the original thread
should be closed as it no longer needs to be open.

This change makes the function POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..cf94b4491f9 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -37,16 +37,18 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
+		return EINVAL;
+	default:
+		/* the function failed, so do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v6 2/2] prep

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index cf94b4491f9..fc88ce20e2b 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
-- 
gitgitgadget

[PATCH v7 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: close handles of threads that have been joined
  prep

 compat/win32/pthread.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v7
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v6:

 -:  ----------- > 1:  b40287508df win32: close handles of threads that have been joined
 1:  0b5afdb93db ! 2:  f780ed525eb win32: close handles of threads that have been joined
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32: close handles of threads that have been joined
     -
     -    After joining threads, the handle to the original thread
     -    should be closed as it no longer needs to be open.
     -
     -    This change makes the function POSIX compliant.
     +    prep
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
       ## compat/win32/pthread.c ##
     +@@ compat/win32/pthread.c: static unsigned __stdcall win32_start_routine(void *arg)
     + }
     + 
     + int pthread_create(pthread_t *thread, const void *unused,
     +-		   void *(*start_routine)(void*), void *arg)
     ++		   void *(*start_routine)(void *), void *arg)
     + {
     + 	thread->arg = arg;
     + 	thread->start_routine = start_routine;
     +-	thread->handle = (HANDLE)
     +-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
     ++	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
     ++						thread, 0, NULL);
     + 
     + 	if (!thread->handle)
     + 		return errno;
      @@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused,
       
       int win32_pthread_join(pthread_t *thread, void **value_ptr)
     @@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused
      -		case WAIT_OBJECT_0:
      -			if (value_ptr)
      -				*value_ptr = thread->arg;
     +-			CloseHandle(thread->handle);
      -			return 0;
      -		case WAIT_ABANDONED:
     +-			CloseHandle(thread->handle);
      -			return EINVAL;
      -		default:
     +-			/* the function failed, so do not detach */
      -			return err_win_to_posix(GetLastError());
      +	switch (WaitForSingleObject(thread->handle, INFINITE)) {
      +	case WAIT_OBJECT_0:
 2:  5b35362e5d2 < -:  ----------- prep

-- 
gitgitgadget

[PATCH v7 1/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..21c705778b6 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 		case WAIT_OBJECT_0:
 			if (value_ptr)
 				*value_ptr = thread->arg;
+			CloseHandle(thread->handle);
 			return 0;
 		case WAIT_ABANDONED:
+			CloseHandle(thread->handle);
 			return EINVAL;
 		default:
+			/* the function failed, so do not detach */
 			return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

Re: [PATCH v7 1/2] win32: close handles of threads that have been joined

[Johannes Sixt]

Am 21.12.22 um 05:46 schrieb Seija Kijin via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
> 
> After the thread terminates, the handle to the
> original thread should be closed.
> 
> This change makes win32_pthread_join POSIX compliant.
> 
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>  compat/win32/pthread.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 2e7eead42cb..21c705778b6 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  		case WAIT_OBJECT_0:
>  			if (value_ptr)
>  				*value_ptr = thread->arg;
> +			CloseHandle(thread->handle);
>  			return 0;
>  		case WAIT_ABANDONED:
> +			CloseHandle(thread->handle);
>  			return EINVAL;
>  		default:
> +			/* the function failed, so do not detach */
>  			return err_win_to_posix(GetLastError());
>  	}
>  }

This patch looks good. It passes the test suite on Windows. Hence:

Acked-by: Johannes Sixt <j6t@kdbg.org>

The follow-up patch in this thread, though, has an incomplete subject
line and no motivation. It is not ready to be picked up.

-- Hannes

[PATCH v7 2/2] prep

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 21c705778b6..fc88ce20e2b 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -37,19 +37,18 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			CloseHandle(thread->handle);
-			return 0;
-		case WAIT_ABANDONED:
-			CloseHandle(thread->handle);
-			return EINVAL;
-		default:
-			/* the function failed, so do not detach */
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
+		return EINVAL;
+	default:
+		/* the function failed, so do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v8] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
    win32: close handles of threads that have been joined
    
    After joining threads, the handle to the original thread should be
    closed as it no longer needs to be open.
    
    Signed-off-by: Seija Kijin doremylover123@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v8
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v7:

 1:  b40287508df = 1:  70588032eb3 win32: close handles of threads that have been joined
 2:  f780ed525eb < -:  ----------- prep


 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..21c705778b6 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 		case WAIT_OBJECT_0:
 			if (value_ptr)
 				*value_ptr = thread->arg;
+			CloseHandle(thread->handle);
 			return 0;
 		case WAIT_ABANDONED:
+			CloseHandle(thread->handle);
 			return EINVAL;
 		default:
+			/* the function failed, so do not detach */
 			return err_win_to_posix(GetLastError());
 	}
 }

base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
-- 
gitgitgadget

[PATCH v9 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: close handles of threads that have been joined
  prep: reformat pthread.c to fit coding guidelines

 compat/win32/pthread.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v9
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v9
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v8:

 1:  70588032eb3 ! 1:  d9b1df22e03 win32: close handles of threads that have been joined
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +			CloseHandle(thread->handle);
       			return EINVAL;
       		default:
     -+			/* the function failed, so do not detach */
     ++			/* the wait failed, so do not detach */
       			return err_win_to_posix(GetLastError());
       	}
       }
 -:  ----------- > 2:  4c82a16a995 prep: reformat pthread.c to fit coding guidelines

-- 
gitgitgadget

[PATCH v9 1/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..81178ed93b7 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 		case WAIT_OBJECT_0:
 			if (value_ptr)
 				*value_ptr = thread->arg;
+			CloseHandle(thread->handle);
 			return 0;
 		case WAIT_ABANDONED:
+			CloseHandle(thread->handle);
 			return EINVAL;
 		default:
+			/* the wait failed, so do not detach */
 			return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

[PATCH v9 2/2] prep: reformat pthread.c to fit coding guidelines

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

The motivation for this change is that
the post-image better fits the coding
guidelines, especially since this file
was changed.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 81178ed93b7..83e088dff0a 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -37,19 +37,18 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			CloseHandle(thread->handle);
-			return 0;
-		case WAIT_ABANDONED:
-			CloseHandle(thread->handle);
-			return EINVAL;
-		default:
-			/* the wait failed, so do not detach */
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
+		return 0;
+	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
+		return EINVAL;
+	default:
+		/* the wait failed, so do not detach */
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

Re: [PATCH v9 2/2] prep: reformat pthread.c to fit coding guidelines

[Junio C Hamano]

"Seija Kijin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Subject: Re: [PATCH v9 2/2] prep: reformat pthread.c to fit coding guidelines

I am guessing that you meant to have "prep" stand for "preparation"
(which by the way is not a good thing to have instead of <area>:
prefix).  Perhaps "windows-pthread:" or something?

In any case, this step should be done first, i.e. [1/2] of a
two-patch series, as a preparation for the real change, I would
think.

[PATCH v10 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32-pthread: prepare pthread.c for change by formatting
  win32: close handles of threads that have been joined

 compat/win32/pthread.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v10
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v10
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v9:

 2:  4c82a16a995 ! 1:  e0cc43efc6c prep: reformat pthread.c to fit coding guidelines
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    prep: reformat pthread.c to fit coding guidelines
     +    win32-pthread: prepare pthread.c for change by formatting
      
     -    The motivation for this change is that
     -    the post-image better fits the coding
     -    guidelines, especially since this file
     -    was changed.
     +    File has been formatted to meet coding guidelines
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
     @@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused
      -		case WAIT_OBJECT_0:
      -			if (value_ptr)
      -				*value_ptr = thread->arg;
     --			CloseHandle(thread->handle);
      -			return 0;
      -		case WAIT_ABANDONED:
     --			CloseHandle(thread->handle);
      -			return EINVAL;
      -		default:
     --			/* the wait failed, so do not detach */
      -			return err_win_to_posix(GetLastError());
      +	switch (WaitForSingleObject(thread->handle, INFINITE)) {
      +	case WAIT_OBJECT_0:
      +		if (value_ptr)
      +			*value_ptr = thread->arg;
     -+		CloseHandle(thread->handle);
      +		return 0;
      +	case WAIT_ABANDONED:
     -+		CloseHandle(thread->handle);
      +		return EINVAL;
      +	default:
     -+		/* the wait failed, so do not detach */
      +		return err_win_to_posix(GetLastError());
       	}
       }
 1:  d9b1df22e03 ! 2:  d05d4c2e4b2 win32: close handles of threads that have been joined
     @@ Commit message
      
       ## compat/win32/pthread.c ##
      @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_ptr)
     - 		case WAIT_OBJECT_0:
     - 			if (value_ptr)
     - 				*value_ptr = thread->arg;
     -+			CloseHandle(thread->handle);
     - 			return 0;
     - 		case WAIT_ABANDONED:
     -+			CloseHandle(thread->handle);
     - 			return EINVAL;
     - 		default:
     -+			/* the wait failed, so do not detach */
     - 			return err_win_to_posix(GetLastError());
     + 	case WAIT_OBJECT_0:
     + 		if (value_ptr)
     + 			*value_ptr = thread->arg;
     ++		CloseHandle(thread->handle);
     + 		return 0;
     + 	case WAIT_ABANDONED:
     ++		CloseHandle(thread->handle);
     + 		return EINVAL;
     + 	default:
     ++		/* the wait failed, so do not detach */
     + 		return err_win_to_posix(GetLastError());
       	}
       }

-- 
gitgitgadget

[PATCH v10 1/2] win32-pthread: prepare pthread.c for change by formatting

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..65e15a560d5 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -37,16 +37,15 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		return 0;
+	case WAIT_ABANDONED:
+		return EINVAL;
+	default:
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v10 2/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 65e15a560d5..83e088dff0a 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -41,10 +41,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 	case WAIT_OBJECT_0:
 		if (value_ptr)
 			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
 		return 0;
 	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
 		return EINVAL;
 	default:
+		/* the wait failed, so do not detach */
 		return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

[PATCH v11 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32-pthread: prepare pthread.c for change by formatting
  win32: close handles of threads that have been joined

 compat/win32/pthread.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v11
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v11
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v10:

 1:  e0cc43efc6c = 1:  e0cc43efc6c win32-pthread: prepare pthread.c for change by formatting
 2:  d05d4c2e4b2 ! 2:  c5d2ddad166 win32: close handles of threads that have been joined
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +		CloseHandle(thread->handle);
       		return EINVAL;
       	default:
     +-		return err_win_to_posix(GetLastError());
      +		/* the wait failed, so do not detach */
     - 		return err_win_to_posix(GetLastError());
     ++		errno = err_win_to_posix(GetLastError());
     ++		return errno;
       	}
       }
     + 

-- 
gitgitgadget

[PATCH v11 1/2] win32-pthread: prepare pthread.c for change by formatting

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..65e15a560d5 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -37,16 +37,15 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		return 0;
+	case WAIT_ABANDONED:
+		return EINVAL;
+	default:
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v11 2/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 65e15a560d5..f63f65c9dfe 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -41,11 +41,15 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 	case WAIT_OBJECT_0:
 		if (value_ptr)
 			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
 		return 0;
 	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
 		return EINVAL;
 	default:
-		return err_win_to_posix(GetLastError());
+		/* the wait failed, so do not detach */
+		errno = err_win_to_posix(GetLastError());
+		return errno;
 	}
 }
 
-- 
gitgitgadget

Re: [PATCH v11 2/2] win32: close handles of threads that have been joined

[René Scharfe]

Am 23.12.22 um 20:19 schrieb Seija Kijin via GitGitGadget:
> From: Seija Kijin <doremylover123@gmail.com>
>
> After the thread terminates, the handle to the
> original thread should be closed.
>
> This change makes win32_pthread_join POSIX compliant.
>
> Signed-off-by: Seija Kijin <doremylover123@gmail.com>
> ---
>  compat/win32/pthread.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 65e15a560d5..f63f65c9dfe 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -41,11 +41,15 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  	case WAIT_OBJECT_0:
>  		if (value_ptr)
>  			*value_ptr = thread->arg;
> +		CloseHandle(thread->handle);
>  		return 0;
>  	case WAIT_ABANDONED:
> +		CloseHandle(thread->handle);
>  		return EINVAL;
>  	default:
> -		return err_win_to_posix(GetLastError());
> +		/* the wait failed, so do not detach */
> +		errno = err_win_to_posix(GetLastError());
> +		return errno;

Why do we need to set errno?  That's the only difference to v10, but I
didn't find an explanation.  POSIX only mentions the return value:
https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_join.html

>  	}
>  }
>

[PATCH v12 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32-pthread: prepare pthread.c for change by formatting
  win32: close handles of threads that have been joined

 compat/win32/pthread.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)


base-commit: 7c2ef319c52c4997256f5807564523dfd4acdfc7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v12
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v12
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v11:

 1:  e0cc43efc6c = 1:  e0cc43efc6c win32-pthread: prepare pthread.c for change by formatting
 2:  c5d2ddad166 ! 2:  fa5cbfa5e0c win32: close handles of threads that have been joined
     @@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_p
      +		CloseHandle(thread->handle);
       		return EINVAL;
       	default:
     --		return err_win_to_posix(GetLastError());
      +		/* the wait failed, so do not detach */
     -+		errno = err_win_to_posix(GetLastError());
     -+		return errno;
     + 		return err_win_to_posix(GetLastError());
       	}
       }
     - 

-- 
gitgitgadget

[PATCH v12 1/2] win32-pthread: prepare pthread.c for change by formatting

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..65e15a560d5 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -37,16 +37,15 @@ int pthread_create(pthread_t *thread, const void *unused,
 
 int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
-	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
-	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		return 0;
+	case WAIT_ABANDONED:
+		return EINVAL;
+	default:
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

Re: [PATCH v12 1/2] win32-pthread: prepare pthread.c for change by formatting

[Junio C Hamano]

"Seija Kijin via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  int pthread_create(pthread_t *thread, const void *unused,
> -		   void *(*start_routine)(void*), void *arg)
> +		   void *(*start_routine)(void *), void *arg)

Good.

>  {
>  	thread->arg = arg;
>  	thread->start_routine = start_routine;
> -	thread->handle = (HANDLE)
> -		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
> +	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
> +						thread, 0, NULL);

I would say this is good either way---preimage may slightly be
easier to read, but it is not worth a patch churn.

> -	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
> -	switch (result) {
> -		case WAIT_OBJECT_0:
> -			if (value_ptr)
> -				*value_ptr = thread->arg;
> -			return 0;
> -		case WAIT_ABANDONED:
> -			return EINVAL;
> -		default:
> -			return err_win_to_posix(GetLastError());
> +	switch (WaitForSingleObject(thread->handle, INFINITE)) {
> +	case WAIT_OBJECT_0:
> +		if (value_ptr)
> +			*value_ptr = thread->arg;
> +		return 0;
> +	case WAIT_ABANDONED:
> +		return EINVAL;
> +	default:
> +		return err_win_to_posix(GetLastError());
>  	}
>  }

Loss of a single-use variable is, strictly speaking, a "while at it"
change that has nothing to do with "reformatting", but otherwise the
result of the hunk looks good.

Will queue.

Thanks.



P.S.  An address that is designed to bounce or sent to black hole
appears on the CC: of the original e-mail message.  Can you stop
doing that (or is that what GGG does without user's control)?

  Rose <83477269+AtariDreams@users.noreply.github.com>

[PATCH v12 2/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 65e15a560d5..83e088dff0a 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -41,10 +41,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 	case WAIT_OBJECT_0:
 		if (value_ptr)
 			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
 		return 0;
 	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
 		return EINVAL;
 	default:
+		/* the wait failed, so do not detach */
 		return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

Re: [PATCH v12 2/2] win32: close handles of threads that have been joined

[Junio C Hamano]

"Seija Kijin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
> index 65e15a560d5..83e088dff0a 100644
> --- a/compat/win32/pthread.c
> +++ b/compat/win32/pthread.c
> @@ -41,10 +41,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
>  	case WAIT_OBJECT_0:
>  		if (value_ptr)
>  			*value_ptr = thread->arg;
> +		CloseHandle(thread->handle);
>  		return 0;
>  	case WAIT_ABANDONED:
> +		CloseHandle(thread->handle);
>  		return EINVAL;
>  	default:
> +		/* the wait failed, so do not detach */
>  		return err_win_to_posix(GetLastError());
>  	}
>  }

This matches what got an Ack from J6t in v7, so I'll queue with
Acked-by: added.  Thanks.

[PATCH v13 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: prepare pthread.c for change by formatting
  win32: close handles of threads that have been joined

 compat/win32/pthread.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)


base-commit: 2b4f5a4e4bb102ac8d967cea653ed753b608193c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v13
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v13
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v12:

 1:  e0cc43efc6c ! 1:  6f89b120641 win32-pthread: prepare pthread.c for change by formatting
     @@ Metadata
      Author: Seija Kijin <doremylover123@gmail.com>
      
       ## Commit message ##
     -    win32-pthread: prepare pthread.c for change by formatting
     +    win32: prepare pthread.c for change by formatting
      
     -    File has been formatted to meet coding guidelines
     +    File has been formatted to meet coding guidelines.
      
          Signed-off-by: Seija Kijin <doremylover123@gmail.com>
      
     @@ compat/win32/pthread.c: static unsigned __stdcall win32_start_routine(void *arg)
       
       	if (!thread->handle)
       		return errno;
     -@@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused,
     - 
     - int win32_pthread_join(pthread_t *thread, void **value_ptr)
     +@@ compat/win32/pthread.c: int win32_pthread_join(pthread_t *thread, void **value_ptr)
       {
     --	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
     --	switch (result) {
     + 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
     + 	switch (result) {
      -		case WAIT_OBJECT_0:
      -			if (value_ptr)
      -				*value_ptr = thread->arg;
     @@ compat/win32/pthread.c: int pthread_create(pthread_t *thread, const void *unused
      -			return EINVAL;
      -		default:
      -			return err_win_to_posix(GetLastError());
     -+	switch (WaitForSingleObject(thread->handle, INFINITE)) {
      +	case WAIT_OBJECT_0:
      +		if (value_ptr)
      +			*value_ptr = thread->arg;
 2:  fa5cbfa5e0c = 2:  f126d6416d5 win32: close handles of threads that have been joined

-- 
gitgitgadget

[PATCH v13 1/2] win32: prepare pthread.c for change by formatting

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

File has been formatted to meet coding guidelines.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..cf53bc61d82 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -39,14 +39,14 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		return 0;
+	case WAIT_ABANDONED:
+		return EINVAL;
+	default:
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v13 2/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index cf53bc61d82..85f8f7920ce 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 	case WAIT_OBJECT_0:
 		if (value_ptr)
 			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
 		return 0;
 	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
 		return EINVAL;
 	default:
+		/* the wait failed, so do not detach */
 		return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

[PATCH v14 0/2] win32: close handles of threads that have been joined

[Rose via GitGitGadget]

After joining threads, the handle to the original thread should be closed as
it no longer needs to be open.

Signed-off-by: Seija Kijin doremylover123@gmail.com

Seija Kijin (2):
  win32: prepare pthread.c for change by formatting
  win32: close handles of threads that have been joined

 compat/win32/pthread.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)


base-commit: 4dbebc36b0893f5094668ddea077d0e235560b16
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v14
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v14
Pull-Request: https://github.com/git/git/pull/1406

Range-diff vs v13:

 1:  6f89b120641 = 1:  a20eafb9541 win32: prepare pthread.c for change by formatting
 2:  f126d6416d5 = 2:  aa0319bc08e win32: close handles of threads that have been joined

-- 
gitgitgadget

[PATCH v14 1/2] win32: prepare pthread.c for change by formatting

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

File has been formatted to meet coding guidelines.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index 2e7eead42cb..cf53bc61d82 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -22,12 +22,12 @@ static unsigned __stdcall win32_start_routine(void *arg)
 }
 
 int pthread_create(pthread_t *thread, const void *unused,
-		   void *(*start_routine)(void*), void *arg)
+		   void *(*start_routine)(void *), void *arg)
 {
 	thread->arg = arg;
 	thread->start_routine = start_routine;
-	thread->handle = (HANDLE)
-		_beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL);
+	thread->handle = (HANDLE)_beginthreadex(NULL, 0, win32_start_routine,
+						thread, 0, NULL);
 
 	if (!thread->handle)
 		return errno;
@@ -39,14 +39,14 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 {
 	DWORD result = WaitForSingleObject(thread->handle, INFINITE);
 	switch (result) {
-		case WAIT_OBJECT_0:
-			if (value_ptr)
-				*value_ptr = thread->arg;
-			return 0;
-		case WAIT_ABANDONED:
-			return EINVAL;
-		default:
-			return err_win_to_posix(GetLastError());
+	case WAIT_OBJECT_0:
+		if (value_ptr)
+			*value_ptr = thread->arg;
+		return 0;
+	case WAIT_ABANDONED:
+		return EINVAL;
+	default:
+		return err_win_to_posix(GetLastError());
 	}
 }
 
-- 
gitgitgadget

[PATCH v14 2/2] win32: close handles of threads that have been joined

[Seija Kijin via GitGitGadget]

From: Seija Kijin <doremylover123@gmail.com>

After the thread terminates, the handle to the
original thread should be closed.

This change makes win32_pthread_join POSIX compliant.

Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
 compat/win32/pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c
index cf53bc61d82..85f8f7920ce 100644
--- a/compat/win32/pthread.c
+++ b/compat/win32/pthread.c
@@ -42,10 +42,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
 	case WAIT_OBJECT_0:
 		if (value_ptr)
 			*value_ptr = thread->arg;
+		CloseHandle(thread->handle);
 		return 0;
 	case WAIT_ABANDONED:
+		CloseHandle(thread->handle);
 		return EINVAL;
 	default:
+		/* the wait failed, so do not detach */
 		return err_win_to_posix(GetLastError());
 	}
 }
-- 
gitgitgadget

Re: [PATCH v14 0/2] win32: close handles of threads that have been joined

[Junio C Hamano]

"Rose via GitGitGadget" <gitgitgadget@gmail.com> writes:

> After joining threads, the handle to the original thread should be closed as
> it no longer needs to be open.
>
> Signed-off-by: Seija Kijin doremylover123@gmail.com
>
> Seija Kijin (2):
>   win32: prepare pthread.c for change by formatting
>   win32: close handles of threads that have been joined
>
>  compat/win32/pthread.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
>
> base-commit: 4dbebc36b0893f5094668ddea077d0e235560b16
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1406%2FAtariDreams%2Fjoin-v14
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1406/AtariDreams/join-v14
> Pull-Request: https://github.com/git/git/pull/1406
>
> Range-diff vs v13:
>
>  1:  6f89b120641 = 1:  a20eafb9541 win32: prepare pthread.c for change by formatting
>  2:  f126d6416d5 = 2:  aa0319bc08e win32: close handles of threads that have been joined


In general, the cover letter of a reroll should explain why
reviewers are asked to look at the updated version instead of the
older one, i.e. what changed since the previous iteration and why.
For those who followed and/or participated in the review of previous
iterations, the range-diff might be sufficient to guess why these
changes were made in order to improve what, but for those who are
coming late to the party, it would be helpful if you explain what
you changed, how and why.

This is true especially if (1) you are rerolling even though the
previous round got little reviews or (2) you are resending without
any change.

Thanks.