Undo Wasm threading fix on panic context tracking #1107
Merged
Conversation
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
Turns out we only need to ensure we don't pass `atomics`-related target feature flags when building for nothreads, and thread locals work properly. Partial backout of 2d84322
PgBiel
changed the title
PgBiel
changed the title
|
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1107 |
Bromeon
added
quality-of-life
This was referenced Mar 30, 2025
Bromeon
approved these changes
Mar 30, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
CI failed here: I wonder if that's a spurious issue? Do you have any idea what could cause this, or if it might be related to this PR? |
|
This PR only makes the panic context tracking code run on Wasm nothreads on debug mode. If that test does not use Wasm nothreads, then there should be no relation (and indeed, CI succeeded before). Perhaps you could try another run |
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.
Partially reverts #1093. The wasm binary name feature is still needed, and in principle we can keep the
#[cfg]to not check for the main thread id as an optimization, though we can revise this in the future if the lack of the main thread id facilities ever becomes a hurdle for other parts of gdext.As for thread locals, turns out we only need to ensure we don't pass
atomics-related target feature flags when building for nothreads, and thread locals start working properly.So, instead of trying to work around them in #1105, we should use thread locals like normal and instead change the book's instructions to suggest removing those flags only for nothreads builds (threaded builds still need them).