[issue44436] [Windows] _thread.start_new_thread() should close the thread handle

[STINNER Victor]

STINNER Victor  added the comment:

Eryk:
> We already close the handle in PyThread_start_new_thread() in 
> Python/thread_nt.h

Oh right! I completely missed this call. Thanks for double checking the issue 
;-) I close the issue as "not a bug".

--
resolution:  -> not a bug
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44436] [Windows] _thread.start_new_thread() should close the thread handle

[STINNER Victor]

STINNER Victor  added the comment:

> [Windows] _thread.start_new_thread() should close the thread handle

So far, I'm not convinced that Python must be changed.

I modified my Python locally so thread_run() writes GetCurrentThread() to 
stderr.

Example:
---
SLEEP = 5

import ctypes
import _thread
import time
import os

HANDLE = ctypes.c_voidp
DWORD = ctypes.c_ulong

ResumeThread = ctypes.windll.kernel32.ResumeThread
ResumeThread.argtypes = (HANDLE,)
ResumeThread.restype = DWORD

SuspendThread = ctypes.windll.kernel32.SuspendThread
SuspendThread.argtypes = (HANDLE,)
SuspendThread.restype = DWORD

def func():
time.sleep(SLEEP)
print("--thread exit--")

def check_handle(handle):
x = SuspendThread(handle)
print(f"SuspendThread({handle}) -> {x}")
x = ResumeThread(handle)
print(f"ResumeThread({handle}) -> {x}")

handle = _thread.start_new_thread(func, ())
print(f"start_new_thread() -> {handle}")
check_handle(handle)
time.sleep(SLEEP+1)
check_handle(handle)
---


Output:
---
start_new_thread() -> 2436
SuspendThread(2436) -> 4294967295
ResumeThread(2436) -> 4294967295
thread_run: GetCurrentThread() -> -2
--thread exit--
SuspendThread(2436) -> 4294967295
ResumeThread(2436) -> 4294967295
---

It seems like the handle returned by _beginthreadex() cannot be used with 
SuspendThread() or ResumeThread(): both fail. GetHandleInformation() also fails 
on this handle (I tried os.get_handle_inheritable()).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44436] [Windows] _thread.start_new_thread() should close the thread handle

[STINNER Victor]

STINNER Victor  added the comment:

> CloseHandle(GetCurrentThread())

This is useless. GetCurrentThread() returns a pseudo handle. The 
GetCurrentThread() documentation says:

"The pseudo handle need not be closed when it is no longer needed. Calling the 
CloseHandle function with this handle has no effect."

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44436] [Windows] _thread.start_new_thread() should close the thread handle

[Eryk Sun]

Eryk Sun  added the comment:

> Would it be safe to close the handle just after PyThread_start_new_thread() 
> success?

We already close the handle in PyThread_start_new_thread() in 
Python/thread_nt.h:

if (hThread == 0) {
// ...
}
else {
dprintf(("%lu: PyThread_start_new_thread succeeded: %p\n",
 PyThread_get_thread_ident(), (void*)hThread));
CloseHandle(hThread);
}

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44436] [Windows] _thread.start_new_thread() should close the thread handle

[STINNER Victor]

New submission from STINNER Victor :

_thread.start_new_thread() is implemented by calling _beginthreadex(). 

Currently, when the thread completes: PyThread_exit_thread() is called which 
calls "_endthreadex(0)" on Windows. I proposed to no longer call it explicitly 
in bpo-44434.

_endthreadex() documentation says that the thread handle must be closed 
explicitly (with CloseHandle()), same applies for ExitThread().

"Unlike _endthread, the _endthreadex function does not close the thread handle, 
thereby allowing other threads to block on this one without fear that the 
handle will be freed out from under the system."

_endthread() closes the thread handle, but Python uses _endthreadex().

Should Python be modified to close explicitly the thread handle once the thread 
terminated?

_endthread() and _endthreadex() documentation:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/endthread-endthreadex?view=msvc-160

Example closing the thread handle in the thread which created the thread:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/beginthread-beginthreadex?view=msvc-160

Simplified code:
---
hThread = (HANDLE)_beginthreadex( NULL, 0, , NULL, 0, 
 );
WaitForSingleObject( hThread, INFINITE );
CloseHandle( hThread );
---


Would it be safe to close the handle just after PyThread_start_new_thread() 
success?

Or should it be done in the C function thread_run(), just before existing, 
CloseHandle(GetCurrentThread())?


To debug this issue, GetHandleInformation() can be used to check if the handle 
is still open or not. For example, call os.get_handle_inheritable(handle) in 
Python.

--
components: Windows
messages: 395939
nosy: eryksun, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
priority: normal
severity: normal
status: open
title: [Windows] _thread.start_new_thread() should close the thread handle
versions: Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com