New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Windows] _thread.start_new_thread() should close the thread handle #88602
Comments
_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: Example closing the thread handle in the thread which created the thread: Simplified code: hThread = (HANDLE)_beginthreadex( NULL, 0, &SecondThreadFunc, NULL, 0, &threadID );
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. |
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);
} |
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." |
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: 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()). |
Eryk:
Oh right! I completely missed this call. Thanks for double checking the issue ;-) I close the issue as "not a bug". |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: