-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests do not set thread name when concurrency is disabled #70492
Comments
This used to be how libtest operates. Unfortunately, that makes libtest not work in environments where threads cannot be spawned, such as Miri. That's why I made it not spawn threads when parallelism is disabled (#56243). However, this could also be achieved by disabling threads on Line 470 in b76238a
We just try to keep uses of cfg(miri) to a minimum -- when Miri executes other code than "the real thing", we could easily miss UB.
Spawning a thread just to let tests get the test name seems quite awkward as well.^^ |
Hm, can we perhaps set the thread name for the current thread? std doesn't provide a way to do it, but I know e.g. postgres does something like that. In the multithreaded case, are we spawning a dedicated thread per test? That seems potentially quite costly, I would've expected some sort of thread pool (though likely without work stealing or anything fancy). |
I think so: Line 472 in b76238a
|
I think this generally would be a welcome addition to the stdlib. There does not seem to be a good reason why the thread name can't be changed after spawning. |
One challenge is that AFAICT, we are restricted to 15 characters in the naming of the thread; currently I believe libstd just ignores this -- the kernel will silently truncate I guess. "The thread name is a meaningful C language string, whose length is restricted to 16 characters, including the terminating null byte ('\0')." is from the manpage for pthread_setname_np which is what we use. I'm not sure to what extent we care, but we'd probably at least want to put the trailing bytes (vs. the leading bytes), I think? Or maybe this is all not actually a restriction because I can't recall seeing truncated names, just all the documentation alleges that it is... |
I believe the right thing to do in general would be to expose a very rudimentary runtime API for tests. |
I've wanted this from time to time. I think |
libtest: run all tests in their own thread, if supported by the host This reverts the threading changes of rust-lang/rust#56243, which made it so that with `-j1`, the test harness does not spawn any threads. Those changes were done to enable Miri to run the test harness, but Miri supports threads nowadays, so this is no longer needed. Using a thread for each test is useful because the thread's name can be set to the test's name which makes panic messages consistent between `-j1` and `-j2` runs and also a bit more readable. I did not revert the HashMap changes of rust-lang/rust#56243; using a deterministic map seems fine for the test harness and the more deterministic testing is the better. Fixes rust-lang/rust#59122 Fixes rust-lang/rust#70492
Currently there are only workarounds for getting the test name from the current running test. The main one that people use is the thread name, but that does not work when tests are run with
--test-threads=1
.I believe it would be beneficial to spawn a thread anyways so that the test name is discoverable. Alternatively — and maybe that's the better solution — there should be an API to discover the name of the current test.
The text was updated successfully, but these errors were encountered: