Conversation
There was a problem hiding this comment.
Pull request overview
This PR restores a safe thread-initialization handshake to eliminate a race where the newly created thread could observe an unsynchronized __handle (and potentially __tid) before the creator had finished publishing them.
Changes:
- Introduces a startup latch (
__init_done) so the new thread waits until the creator signals initialization completion. - Updates thread startup to block on the latch and assert
__tid/__handlehave been published before running the user procedure. - Adjusts the
CreateThread()thread-id output parameter cast in preparation for the new protocol.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| mcfgthread/thread.h | Adds an internal startup latch field (under __MCF_THREAD_DETAILS) using event storage. |
| mcfgthread/src/thread.c | Implements the “new thread waits for creator” initialization protocol and signals readiness after successful thread creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ca3396f removed the initialization protocol. There were idempotant writes to ensure `__tid` have a correct value before the user-defined thread procedure, but `__handle` was left unsynchronized between the new thread and the caller. The fix is to make the new thread wait for the creator. The creator will not wait for the new thread; if `_MCF_thread_new()` is called in `DllMain()`, it would deadlock.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ca3396f removed the initialization protocol.
There were idempotant writes to ensure
__tidhave a correct value before theuser-defined thread procedure, but
__handlewas left unsynchronized betweenthe new thread and the caller.
The fix is to make the new thread wait for the creator. The creator will not
wait for the new thread; if
_MCF_thread_new()is called inDllMain(), itwould deadlock.