add test for main thread thread-local destructors #126829
Closed
Conversation
This file contains 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
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
rustbot
added
S-waiting-on-review
4 tasks
|
@bors r+ |
bors
added
S-waiting-on-bors
|
somehow it did not occur to me that we could even test this (I didn't remember that |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 22, 2024
…ubilee add test for main thread thread-local destructors Fixes rust-lang#28129
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
...wasm. |
|
don't know if the wasmtime test runner works with println at all... hope it does... |
|
We don't need threads, just thread-locals... but I guess wasm has neither? |
RalfJung
force-pushed
the
main-thread-tls
branch
from
d125429 to
3aadc26
Compare
There are other @bors r=workingjubilee rollup=iffy |
bors
added
S-waiting-on-bors
|
In general libstd lacks thread-local support if the target lacks threading support because the question winds up... mildly incoherent? We could probably still fix this, but it would require some design work for making sure that everything is correctly forward-compatible for the surprisingly common case of "this target without threading support gains threading support". I keep forgetting wasm is technically still in the "lacks threads" bin because there are wasm targets that support threads! Just not the one we run in the test suite... |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 23, 2024
…ubilee add test for main thread thread-local destructors Fixes rust-lang#28129
This comment has been minimized.
This comment has been minimized.
|
💔 Test failed - checks-actions |
bjorn3
reviewed
Jun 24, 2024
RalfJung
force-pushed
the
main-thread-tls
branch
from
e055e1c to
32c648c
Compare
|
@bors retry r=workingjubilee |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 24, 2024
…ubilee add test for main thread thread-local destructors Fixes rust-lang#28129
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
What...?!? It fails on x86_64-unknown-linux-gnu but actually it works in the build distributed by rustup (and in PR CI)? Could this have to do with the dist builders using an ancient glibc or so? |
|
CentOS 7 uses glibc 2.17 which was released in December of 2012. Glibc should have supported it since 2.3.4 (from December of 2004) however. See #126858 (comment). |
|
What. |
|
This time it seems to print "Foo dtor" but not "Bar dtor". Is it just... racy, and the desired test behavior is never guaranteed? |
The POSIX specification explicitly doesn't guarantee a destruction order for |
The bot summary is misleading here, the full log says So, none of the destructors run.
That is true but not relevant here: the second destructor is registered when the first one runs, so the opposite order is impossible. |
|
We only use The comments there say it was only added in glibc 2.18, which I also see in my current symbol versions, I can't get native C to call anything from the main thread either using |
|
Okay, so... we should only run this test if the glibc version on the current host is at least 2.18. How do we do that?^^ |
Oh, thank you. I feel silly now. |
|
@jieyouxu we have a somewhat interesting test requirement. you've become unfortunately familiar with how compiletest and some of our CI works. any ideas? |
Sorry for the slow reply, anyways here's my 2 cents: Since this test seems to want specific glibc versions, you could introduce a compiletest directive Once the glibc requirement is handled, then the test kind itself is less important, it can be a run-make test if you need the flexibility or if it's clearer as a run-make test (e.g. you want to control and demonstrate exact e.g. gcc invocations). |
|
Just need to make sure the test doesn't become never-ran-in-any-ci-jobs-at-all, you can try dockering the linux jobs locally to see if the test runs at all. |
|
@bors r- bors sync |
bors
added
S-waiting-on-author
If you can locate the library, it is also executable as a program, e.g.: Alternatively, we have this rust/library/std/src/sys/pal/unix/os.rs Lines 772 to 794 in 1086aff |
|
why does std parse glibc versions... ...don't tell me, I don't actually want to know. |
|
I'm afraid I won't be able to deal with all these special cases. |
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.
Fixes #28129