win32: close handles of threads that have been joined

From: Seija Kijin <[email protected]>

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

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

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(-)

index 2e7eead42cb..de89667ef70 100644
@@ -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

Am 19.12.22 um 19:34 schrieb Rose via GitGitGadget:

From: Seija Kijin <[email protected]>

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

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

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(-)

index 2e7eead42cb..0d62c45ecc2 100644
@@ -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 <[email protected]>

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

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

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(-)

index 2e7eead42cb..306834a7153 100644
@@ -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.

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 <[email protected]>

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

## 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 <[email protected]>

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

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

index 2e7eead42cb..89e89c3fe00 100644
@@ -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 <[email protected]>

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

index 89e89c3fe00..0831ffbc3ae 100644
@@ -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 <[email protected]>

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.

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

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 <[email protected]>
      
       ## 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 <[email protected]>
      
       ## 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(-)

index 2e7eead42cb..7f8503b4b50 100644
@@ -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:

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.

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

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 <[email protected]>
      
     @@ 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 <[email protected]>

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.

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

index 2e7eead42cb..cf94b4491f9 100644
@@ -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 <[email protected]>

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

index cf94b4491f9..fc88ce20e2b 100644
@@ -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.

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 <[email protected]>
      
       ## 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 <[email protected]>
      
       ## 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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index 2e7eead42cb..21c705778b6 100644
@@ -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

From: Seija Kijin <[email protected]>

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

index 21c705778b6..fc88ce20e2b 100644
@@ -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

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

From: Seija Kijin <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

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(+)

index 2e7eead42cb..21c705778b6 100644
@@ -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.

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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index 2e7eead42cb..81178ed93b7 100644
@@ -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 <[email protected]>

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

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

index 81178ed93b7..83e088dff0a 100644
@@ -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

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.

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

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 <[email protected]>
      
       ## 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 <[email protected]>
      
     @@ 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 <[email protected]>

File has been formatted to meet coding guidelines

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

index 2e7eead42cb..65e15a560d5 100644
@@ -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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index 65e15a560d5..83e088dff0a 100644
@@ -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.

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 <[email protected]>

File has been formatted to meet coding guidelines

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

index 2e7eead42cb..65e15a560d5 100644
@@ -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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

---
 compat/win32/pthread.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 65e15a560d5..f63f65c9dfe 100644
@@ -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:

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

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 <[email protected]>

File has been formatted to meet coding guidelines

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

index 2e7eead42cb..65e15a560d5 100644
@@ -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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index 65e15a560d5..83e088dff0a 100644
@@ -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.

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 <[email protected]>
      
       ## 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 <[email protected]>
      
     @@ 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 <[email protected]>

File has been formatted to meet coding guidelines.

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

index 2e7eead42cb..cf53bc61d82 100644
@@ -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 <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index cf53bc61d82..85f8f7920ce 100644
@@ -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 <[email protected]>

File has been formatted to meet coding guidelines.

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

index 2e7eead42cb..cf53bc61d82 100644
@@ -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

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

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

From: Seija Kijin <[email protected]>

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

This change makes win32_pthread_join POSIX compliant.

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

index cf53bc61d82..85f8f7920ce 100644
@@ -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

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.