Skip to content

Commit 52f0d00

Browse files
committed
Auto merge of rust-lang#146468 - Zalathar:rollup-6u3s44d, r=Zalathar
Rollup of 15 pull requests Successful merges: - rust-lang#144549 (match clang's `va_arg` assembly on arm targets) - rust-lang#145895 (thread parking: fix docs and examples) - rust-lang#146308 (support integer literals in `${concat()}`) - rust-lang#146323 (check before test for hardware capabilites in bits 32~63 of usize) - rust-lang#146332 (tidy: make behavior of extra-checks more uniform) - rust-lang#146374 (Update `browser-ui-test` version to `0.22.2`) - rust-lang#146413 (Improve suggestion in case a bare URL is surrounded by brackets) - rust-lang#146426 (Bump miow to 0.60.1) - rust-lang#146432 (Implement `Socket::take_error` for Hermit) - rust-lang#146433 (rwlock tests: fix miri macos test regression) - rust-lang#146435 (Change the default value of `gcc.download-ci-gcc` to `true`) - rust-lang#146439 (fix cfg for poison test macro) - rust-lang#146448 ([rustdoc] Correctly handle literal search on paths) - rust-lang#146449 (Fix `libgccjit` symlink when we build GCC locally) - rust-lang#146455 (test: remove an outdated normalization for rustc versions) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 2c14fa6 + 2f3c668 commit 52f0d00

File tree

10 files changed

+124
-66
lines changed

10 files changed

+124
-66
lines changed

std/src/sys/net/connection/socket/hermit.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,8 @@ impl Socket {
304304
}
305305

306306
pub fn take_error(&self) -> io::Result<Option<io::Error>> {
307-
unimplemented!()
307+
let raw: c_int = getsockopt(self, libc::SOL_SOCKET, libc::SO_ERROR)?;
308+
if raw == 0 { Ok(None) } else { Ok(Some(io::Error::from_raw_os_error(raw as i32))) }
308309
}
309310

310311
// This is used by sys_common code to abstract over Windows and Unix.

std/src/sys/pal/hermit/os.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::ffi::{OsStr, OsString};
33
use crate::marker::PhantomData;
44
use crate::path::{self, PathBuf};
55
use crate::sys::unsupported;
6-
use crate::{fmt, io, str};
6+
use crate::{fmt, io};
77

88
pub fn errno() -> i32 {
99
unsafe { hermit_abi::get_errno() }

std/src/sys/sync/once/queue.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,9 @@ fn wait(
276276
// If the managing thread happens to signal and unpark us before we
277277
// can park ourselves, the result could be this thread never gets
278278
// unparked. Luckily `park` comes with the guarantee that if it got
279-
// an `unpark` just before on an unparked thread it does not park.
279+
// an `unpark` just before on an unparked thread it does not park. Crucially, we know
280+
// the `unpark` must have happened between the `compare_exchange_weak` above and here,
281+
// and there's no other `park` in that code that could steal our token.
280282
// SAFETY: we retrieved this handle on the current thread above.
281283
unsafe { node.thread.park() }
282284
}

std/src/thread/mod.rs

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,13 +1021,23 @@ impl Drop for PanicGuard {
10211021
/// specifying a maximum time to block the thread for.
10221022
///
10231023
/// * The [`unpark`] method on a [`Thread`] atomically makes the token available
1024-
/// if it wasn't already. Because the token is initially absent, [`unpark`]
1025-
/// followed by [`park`] will result in the second call returning immediately.
1026-
///
1027-
/// The API is typically used by acquiring a handle to the current thread,
1028-
/// placing that handle in a shared data structure so that other threads can
1029-
/// find it, and then `park`ing in a loop. When some desired condition is met, another
1030-
/// thread calls [`unpark`] on the handle.
1024+
/// if it wasn't already. Because the token can be held by a thread even if it is currently not
1025+
/// parked, [`unpark`] followed by [`park`] will result in the second call returning immediately.
1026+
/// However, note that to rely on this guarantee, you need to make sure that your `unpark` happens
1027+
/// after all `park` that may be done by other data structures!
1028+
///
1029+
/// The API is typically used by acquiring a handle to the current thread, placing that handle in a
1030+
/// shared data structure so that other threads can find it, and then `park`ing in a loop. When some
1031+
/// desired condition is met, another thread calls [`unpark`] on the handle. The last bullet point
1032+
/// above guarantees that even if the `unpark` occurs before the thread is finished `park`ing, it
1033+
/// will be woken up properly.
1034+
///
1035+
/// Note that the coordination via the shared data structure is crucial: If you `unpark` a thread
1036+
/// without first establishing that it is about to be `park`ing within your code, that `unpark` may
1037+
/// get consumed by a *different* `park` in the same thread, leading to a deadlock. This also means
1038+
/// you must not call unknown code between setting up for parking and calling `park`; for instance,
1039+
/// if you invoke `println!`, that may itself call `park` and thus consume your `unpark` and cause a
1040+
/// deadlock.
10311041
///
10321042
/// The motivation for this design is twofold:
10331043
///
@@ -1058,33 +1068,47 @@ impl Drop for PanicGuard {
10581068
///
10591069
/// ```
10601070
/// use std::thread;
1061-
/// use std::sync::{Arc, atomic::{Ordering, AtomicBool}};
1071+
/// use std::sync::atomic::{Ordering, AtomicBool};
10621072
/// use std::time::Duration;
10631073
///
1064-
/// let flag = Arc::new(AtomicBool::new(false));
1065-
/// let flag2 = Arc::clone(&flag);
1074+
/// static QUEUED: AtomicBool = AtomicBool::new(false);
1075+
/// static FLAG: AtomicBool = AtomicBool::new(false);
10661076
///
10671077
/// let parked_thread = thread::spawn(move || {
1078+
/// println!("Thread spawned");
1079+
/// // Signal that we are going to `park`. Between this store and our `park`, there may
1080+
/// // be no other `park`, or else that `park` could consume our `unpark` token!
1081+
/// QUEUED.store(true, Ordering::Release);
10681082
/// // We want to wait until the flag is set. We *could* just spin, but using
10691083
/// // park/unpark is more efficient.
1070-
/// while !flag2.load(Ordering::Relaxed) {
1071-
/// println!("Parking thread");
1084+
/// while !FLAG.load(Ordering::Acquire) {
1085+
/// // We can *not* use `println!` here since that could use thread parking internally.
10721086
/// thread::park();
10731087
/// // We *could* get here spuriously, i.e., way before the 10ms below are over!
10741088
/// // But that is no problem, we are in a loop until the flag is set anyway.
1075-
/// println!("Thread unparked");
10761089
/// }
10771090
/// println!("Flag received");
10781091
/// });
10791092
///
10801093
/// // Let some time pass for the thread to be spawned.
10811094
/// thread::sleep(Duration::from_millis(10));
10821095
///
1096+
/// // Ensure the thread is about to park.
1097+
/// // This is crucial! It guarantees that the `unpark` below is not consumed
1098+
/// // by some other code in the parked thread (e.g. inside `println!`).
1099+
/// while !QUEUED.load(Ordering::Acquire) {
1100+
/// // Spinning is of course inefficient; in practice, this would more likely be
1101+
/// // a dequeue where we have no work to do if there's nobody queued.
1102+
/// std::hint::spin_loop();
1103+
/// }
1104+
///
10831105
/// // Set the flag, and let the thread wake up.
1084-
/// // There is no race condition here, if `unpark`
1106+
/// // There is no race condition here: if `unpark`
10851107
/// // happens first, `park` will return immediately.
1108+
/// // There is also no other `park` that could consume this token,
1109+
/// // since we waited until the other thread got queued.
10861110
/// // Hence there is no risk of a deadlock.
1087-
/// flag.store(true, Ordering::Relaxed);
1111+
/// FLAG.store(true, Ordering::Release);
10881112
/// println!("Unpark the thread");
10891113
/// parked_thread.thread().unpark();
10901114
///
@@ -1494,10 +1518,14 @@ impl Thread {
14941518
/// ```
14951519
/// use std::thread;
14961520
/// use std::time::Duration;
1521+
/// use std::sync::atomic::{AtomicBool, Ordering};
1522+
///
1523+
/// static QUEUED: AtomicBool = AtomicBool::new(false);
14971524
///
14981525
/// let parked_thread = thread::Builder::new()
14991526
/// .spawn(|| {
15001527
/// println!("Parking thread");
1528+
/// QUEUED.store(true, Ordering::Release);
15011529
/// thread::park();
15021530
/// println!("Thread unparked");
15031531
/// })
@@ -1506,6 +1534,15 @@ impl Thread {
15061534
/// // Let some time pass for the thread to be spawned.
15071535
/// thread::sleep(Duration::from_millis(10));
15081536
///
1537+
/// // Wait until the other thread is queued.
1538+
/// // This is crucial! It guarantees that the `unpark` below is not consumed
1539+
/// // by some other code in the parked thread (e.g. inside `println!`).
1540+
/// while !QUEUED.load(Ordering::Acquire) {
1541+
/// // Spinning is of course inefficient; in practice, this would more likely be
1542+
/// // a dequeue where we have no work to do if there's nobody queued.
1543+
/// std::hint::spin_loop();
1544+
/// }
1545+
///
15091546
/// println!("Unpark the thread");
15101547
/// parked_thread.thread().unpark();
15111548
///

std/src/thread/tests.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ fn test_park_unpark_called_other_thread() {
287287
for _ in 0..10 {
288288
let th = thread::current();
289289

290+
// Here we rely on `thread::spawn` (specifically the part that runs after spawning
291+
// the thread) to not consume the parking token.
290292
let _guard = thread::spawn(move || {
291293
super::sleep(Duration::from_millis(50));
292294
th.unpark();
@@ -316,6 +318,8 @@ fn test_park_timeout_unpark_called_other_thread() {
316318
for _ in 0..10 {
317319
let th = thread::current();
318320

321+
// Here we rely on `thread::spawn` (specifically the part that runs after spawning
322+
// the thread) to not consume the parking token.
319323
let _guard = thread::spawn(move || {
320324
super::sleep(Duration::from_millis(50));
321325
th.unpark();

std/tests/sync/condvar.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ nonpoison_and_poison_unwrap_test!(
1717
}
1818
);
1919

20-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
20+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
2121
nonpoison_and_poison_unwrap_test!(
2222
name: notify_one,
2323
test_body: {
@@ -38,7 +38,7 @@ nonpoison_and_poison_unwrap_test!(
3838
}
3939
);
4040

41-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
41+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
4242
nonpoison_and_poison_unwrap_test!(
4343
name: notify_all,
4444
test_body: {
@@ -79,7 +79,7 @@ nonpoison_and_poison_unwrap_test!(
7979
}
8080
);
8181

82-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
82+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
8383
nonpoison_and_poison_unwrap_test!(
8484
name: test_mutex_arc_condvar,
8585
test_body: {
@@ -116,7 +116,7 @@ nonpoison_and_poison_unwrap_test!(
116116
}
117117
);
118118

119-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
119+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
120120
nonpoison_and_poison_unwrap_test!(
121121
name: wait_while,
122122
test_body: {
@@ -141,7 +141,7 @@ nonpoison_and_poison_unwrap_test!(
141141
}
142142
);
143143

144-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
144+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
145145
nonpoison_and_poison_unwrap_test!(
146146
name: wait_timeout_wait,
147147
test_body: {
@@ -164,7 +164,7 @@ nonpoison_and_poison_unwrap_test!(
164164
}
165165
);
166166

167-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
167+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
168168
nonpoison_and_poison_unwrap_test!(
169169
name: wait_timeout_while_wait,
170170
test_body: {
@@ -180,7 +180,7 @@ nonpoison_and_poison_unwrap_test!(
180180
}
181181
);
182182

183-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
183+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
184184
nonpoison_and_poison_unwrap_test!(
185185
name: wait_timeout_while_instant_satisfy,
186186
test_body: {
@@ -197,7 +197,7 @@ nonpoison_and_poison_unwrap_test!(
197197
}
198198
);
199199

200-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
200+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
201201
nonpoison_and_poison_unwrap_test!(
202202
name: wait_timeout_while_wake,
203203
test_body: {
@@ -226,7 +226,7 @@ nonpoison_and_poison_unwrap_test!(
226226
}
227227
);
228228

229-
#[cfg_attr(any(target_os = "emscripten", target_os = "wasi"), ignore)] // no threads
229+
#[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] // No threads.
230230
nonpoison_and_poison_unwrap_test!(
231231
name: wait_timeout_wake,
232232
test_body: {

std/tests/sync/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ fn result_unwrap<T, E: std::fmt::Debug>(x: Result<T, E>) -> T {
5858
/// a no-op (the identity function).
5959
///
6060
/// The test names will be prefiex with `poison_` or `nonpoison_`.
61+
///
62+
/// Important: most attributes (except `cfg`) will not work properly! (They are only applied to the first test.)
63+
/// See <https://github.com/rust-lang/rust/pull/146433> for more information.
6164
macro_rules! nonpoison_and_poison_unwrap_test {
6265
(
6366
name: $name:ident,

std/tests/sync/mutex.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ nonpoison_and_poison_unwrap_test!(
266266
}
267267
);
268268

269-
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
269+
#[cfg(panic = "unwind")] // Requires unwinding support.
270270
nonpoison_and_poison_unwrap_test!(
271271
name: test_panics,
272272
test_body: {
@@ -297,7 +297,7 @@ nonpoison_and_poison_unwrap_test!(
297297
}
298298
);
299299

300-
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
300+
#[cfg(panic = "unwind")] // Requires unwinding support.
301301
nonpoison_and_poison_unwrap_test!(
302302
name: test_mutex_arc_access_in_unwind,
303303
test_body: {

std/tests/sync/rwlock.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ nonpoison_and_poison_unwrap_test!(
5050
// FIXME: On macOS we use a provenance-incorrect implementation and Miri
5151
// catches that issue with a chance of around 1/1000.
5252
// See <https://github.com/rust-lang/rust/issues/121950> for details.
53-
#[cfg_attr(all(miri, target_os = "macos"), ignore)]
53+
#[cfg(not(all(miri, target_os = "macos")))]
5454
nonpoison_and_poison_unwrap_test!(
5555
name: frob,
5656
test_body: {
@@ -124,7 +124,7 @@ nonpoison_and_poison_unwrap_test!(
124124
}
125125
);
126126

127-
#[cfg_attr(not(panic = "unwind"), ignore = "test requires unwinding support")]
127+
#[cfg(panic = "unwind")] // Requires unwinding support.
128128
nonpoison_and_poison_unwrap_test!(
129129
name: test_rw_arc_access_in_unwind,
130130
test_body: {
@@ -315,7 +315,7 @@ nonpoison_and_poison_unwrap_test!(
315315

316316
// FIXME: On macOS we use a provenance-incorrect implementation and Miri catches that issue.
317317
// See <https://github.com/rust-lang/rust/issues/121950> for details.
318-
#[cfg_attr(all(miri, target_os = "macos"), ignore)]
318+
#[cfg(not(all(miri, target_os = "macos")))]
319319
nonpoison_and_poison_unwrap_test!(
320320
name: test_downgrade_observe,
321321
test_body: {
@@ -362,7 +362,7 @@ nonpoison_and_poison_unwrap_test!(
362362

363363
// FIXME: On macOS we use a provenance-incorrect implementation and Miri catches that issue.
364364
// See <https://github.com/rust-lang/rust/issues/121950> for details.
365-
#[cfg_attr(all(miri, target_os = "macos"), ignore)]
365+
#[cfg(not(all(miri, target_os = "macos")))]
366366
nonpoison_and_poison_unwrap_test!(
367367
name: test_downgrade_atomic,
368368
test_body: {

0 commit comments

Comments
 (0)