Skip to content

gh-140746: Fix Thread.start() that can hang indefinitely - v2#144750

Open
YvesDup wants to merge 3 commits intopython:mainfrom
YvesDup:thread-hang-memory
Open

gh-140746: Fix Thread.start() that can hang indefinitely - v2#144750
YvesDup wants to merge 3 commits intopython:mainfrom
YvesDup:thread-hang-memory

Conversation

@YvesDup
Copy link
Contributor

@YvesDup YvesDup commented Feb 12, 2026

This PR is based on the @ryv-odoo PR. I would have preferred to start from @ryv-odoo's PR branch rather than duplicating his code, but I don't know if that's possible.

Following the @colesbury comment,

I would lean towards changing Thread.start and _start_joinable_thread so that we can do the waiting in C:

       try:
           # Start joinable thread
           _start_joinable_thread(self._bootstrap, handle=self._os_thread_handle,
                                  daemon=self.daemon)
       except Exception:
           with _active_limbo_lock:
               del _limbo[self]
           raise

I think we could add a new states to _os_thread_handle to represent the state when self._started.set() was called and to represent the state when the thread fails before then.

I submit this PR that includes the waiting in C the module and raise a runtime exception when the thread handle state is set to failed. This exception is treated in the Thread.start.

Copy link
Contributor

@bkap123 bkap123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a couple of small comments on semantics.


// Handles state transitions according to the following diagram:
//
// NOT_STARTED -> STARTING -> RUNNING -> DONE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should be updated now that there can be a FAILED state. Possibly just replace error with FAILED.

// NOT_STARTED -> STARTING -> RUNNING -> DONE
// | ^
// | |
// +----- error --------+
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// +----- FAILED --------+


PyMutex mutex;

PyEvent thread_is_bootstraped;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"bootstraped" is misspelled everywhere (should be bootstrapped).

PyErr_FormatUnraisable(
"Exception ignored in thread started by %R", boot->func);
}
// Notify that the bootstraped is done and failed (e.g. Memory error).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes more sense:

Suggested change
// Notify that the bootstraped is done and failed (e.g. Memory error).
// Notify that bootstrap is done and failed (e.g. Memory error).

@@ -0,0 +1,2 @@
Fix :func:`threading.Thread.start` that can hang indefinitely in case of heap memory exhaustion
during initialization of the new thread.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add something about the new FAILED state that is managed by the C code?

return NULL;
}

// gh-140746: catch error before thread really start
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// gh-140746: catch error before thread really start
// gh-140746: catch error before thread really starts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants