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

From: Seija Kijin <redacted>

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 <redacted>
---
    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

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

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?

Am 19.12.22 um 19:34 schrieb Rose via GitGitGadget:

From: Seija Kijin <redacted>

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 <redacted>
---

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

From: Seija Kijin <redacted>

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 <redacted>
---
    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

From: Seija Kijin <redacted>

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 <redacted>
---
    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

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 [off-list ref]
      
       ## 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 [off-list ref]
      
       ## 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

From: Seija Kijin <redacted>

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
    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 [off-list ref]
      
       ## 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 [off-list ref]
      
       ## 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

Am 20.12.22 um 22:18 schrieb Rose via GitGitGadget:

From: Seija Kijin <redacted>

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 <redacted>
---

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

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 [off-list ref]
      
     @@ 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

From: Seija Kijin <redacted>

Signed-off-by: Seija Kijin <redacted>
---
 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

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 [off-list ref]
      
       ## 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 [off-list ref]
      
       ## 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

From: Seija Kijin <redacted>

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

Am 21.12.22 um 05:46 schrieb Seija Kijin via GitGitGadget:

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

Acked-by: Johannes Sixt <redacted>

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

From: Seija Kijin <redacted>

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 <redacted>
---
    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

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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

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 [off-list ref]
      
       ## 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 [off-list ref]
      
     @@ 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

From: Seija Kijin <redacted>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

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

From: Seija Kijin <redacted>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

Am 23.12.22 um 20:19 schrieb Seija Kijin via GitGitGadget:

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

 	}
 }

From: Seija Kijin <redacted>

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 <redacted>
---
 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

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

From: Seija Kijin <redacted>

File has been formatted to meet coding guidelines

Signed-off-by: Seija Kijin <redacted>
---
 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

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 [off-list ref]
      
       ## 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 [off-list ref]
      
     @@ 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

From: Seija Kijin <redacted>

File has been formatted to meet coding guidelines.

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

From: Seija Kijin <redacted>

File has been formatted to meet coding guidelines.

Signed-off-by: Seija Kijin <redacted>
---
 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

From: Seija Kijin <redacted>

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 <redacted>
---
 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

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