`Thread.is_alive` should only return `False` after a thread exits (once it's started)

[colesbury]

Feature or enhancement

Currently, Thread.is_alive will return False shortly after the run() function finishes, even though the underlying OS thread may still be alive.

In #110829, we made Thread.join call the OS join API so that it waits for the OS thread to actually exit. We should apply the same behavior to Thread.is_alive():

• In some places, we use Thread.is_alive as an alternative to Thread.join. For example, in test_free_threading.test_monitoring, the test exits once all the background threads are no longer "alive", but that may be a little too soon -- the OS threads may still be running, which can lead to false positives in the refleak tests.
• The pattern for Thread.join(timeout=...) requires calling Thread.is_alive afterwards to differentiate timeouts vs. successful joins, but is_alive() may return False "too soon", while the OS thread is still alive.

(Split off from #136692)

cc @mpage @gpshead

Linked PRs

• gh-137017: Fix Thread.is_alive() to only return False after the underlying OS thread exits #137315
• [3.14] gh-137017: Ensure Thread.is_alive() only returns False after the underlying OS thread exits (gh-137315) #138916
• [3.13] gh-137017: Ensure Thread.is_alive() only returns False after the underlying OS thread exits (gh-137315) #138917

[gpshead]

This makes a lot of sense if we can actually get this plumbed right.

[deepwzh]

Can I try to modifying Thread.is_alive to make it return True when the thread hasn't really exited?

[colesbury]

I think it's more subtle than that. I don't think there's a way to determine if a thread has really exited without joining it. So you need to check if the thread is about to exit and in that case join.

Also need to make sure that is_alive on DummyThreads and the MainThread still work appropriately and update the docs.

[Abdoulrasheed]

@colesbury can we update PyThreadHandleObject_is_done in _threadmodule.c to join the thread?

Edited Diff

 static PyObject *
 PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(dummy)){
     PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
     if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) {
-        Py_RETURN_TRUE;
+        if (ThreadHandle_join(self->handle, -1) == 0) {
+            Py_RETURN_TRUE;
+        }
     }
     else {
         Py_RETURN_FALSE;
     }
 }

static PyObject *
PyThreadHandleObject_is_done(PyObject *op, PyObject *Py_UNUSED(dummy))
{
    PyThreadHandleObject *self = PyThreadHandleObject_CAST(op);
    if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) {
+      if (self->handle->ident != PyThread_get_thread_ident()) {
+          (void)ThreadHandle_join(self->handle, -1);
+      }
        Py_RETURN_TRUE;
    }
    else {
        Py_RETURN_FALSE;
    }
}

[gpshead]

While it ordinarily feels surprising to trigger a potentially blocking join from a boolean .is_alive() method... I don't think that join is blocking in practice in the scenarios where this could be called, it looks like we only set .thread_is_exiting (via PyNotify) in scenarios where we are not doing any further calls that might meaningfully block. (I'm ignoring any potential for malloc to block as irrelevant)

[colesbury]

@Abdoulrasheed, I think it should be enough to do the following, instead of ThreadHandle_join:

if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) {
    if (_PyOnceFlag_CallOnce(&self->once, join_thread, self) == -1) {
        return -1;
    }
    Py_RETURN_TRUE;
}

... I don't think that join is blocking in practice in the scenarios where this could be called

Yeah, I agree both that it feels a bit weird and I also don't think it should block in practice.

We'll need to be careful about the main thread and "dummy" threads (i.e., threads created from C/C++ outside the threading module), since they have different behaviors.

[Abdoulrasheed]

Thanks @colesbury. I'll update and open a PR.

[Abdoulrasheed]

@colesbury The function is expected to return a PyObject*. I assume you meant to return NULL and set an appropriate exception, rather than return -1?

I've updated it to return NULL instead.