Title: [Windows] _thread.start_new_thread() should close the thread handle
Type: Stage: resolved
Components: Windows Versions: Python 3.11
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, paul.moore, steve.dower, tim.golden, vstinner, zach.ware
Priority: normal Keywords:

Created on 2021-06-16 15:43 by vstinner, last changed 2021-06-16 16:03 by vstinner. This issue is now closed.

Messages (5)
msg395939 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-06-16 15:43
_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.
msg395944 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-06-16 15:56
> 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));
msg395945 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-06-16 15:57
> 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."
msg395947 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-06-16 16:01
> [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.


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():
    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}")

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()).
msg395948 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-06-16 16:03
> 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".
Date User Action Args
2021-06-16 16:03:07vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg395948

stage: resolved
2021-06-16 16:01:21vstinnersetmessages: + msg395947
2021-06-16 15:57:49vstinnersetmessages: + msg395945
2021-06-16 15:56:25eryksunsetmessages: + msg395944
2021-06-16 15:43:23vstinnercreate