std: Audit std::thread implementations #24447
Conversation
|
r? @aturon |
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #24426) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
audit-thread
branch
from
3cdb1e2 to
ed911ec
Compare
|
☔ The latest upstream changes (presumably #24674) made this pull request unmergeable. Please resolve the merge conflicts. |
alexcrichton
force-pushed
the
audit-thread
branch
from
ed911ec to
2bf1e16
Compare
|
|
||
| unsafe impl<T:Send> Send for Packet<T> {} | ||
| unsafe impl<T> Sync for Packet<T> {} | ||
| type Packet<T> = Arc<Mutex<Option<Result<T>>>>; |
There was a problem hiding this comment.
Why introduce a mutex? The use of join should already guarantee sufficient synchronization.
There was a problem hiding this comment.
This helped avoid a number of unsafe impl blocks as well as removing a bit of unsafe code. I definitely agree that it was properly synchronized before, I was just hoping to cut down on the amount of unsafe here. I'll see if I can't finesse a nicer solution to this though.
There was a problem hiding this comment.
OK. Not a huge deal, the extra call into system APIs would probably never show up on a profile.
|
This looks good to me, though I am curious about the introduction of a |
Much of this code hasn't been updated in quite some time and this commit does a small audit of the functionality: * Implementation functions now centralize all functionality on a locally defined `Thread` type. * The `detach` method has been removed in favor of a `Drop` implementation. This notably fixes leaking thread handles on Windows. * The `Thread` structure is now appropriately annotated with `Send` and `Sync` automatically on Windows and in a custom fashion on Unix. * The unsafety of creating a thread has been pushed out to the right boundaries now. Closes rust-lang#24442
alexcrichton
force-pushed
the
audit-thread
branch
from
2bf1e16 to
2e11009
Compare
|
@bors: r=aturon |
|
📌 Commit 2e11009 has been approved by |
Much of this code hasn't been updated in quite some time and this commit does a small audit of the functionality: * Implementation functions now centralize all functionality on a locally defined `Thread` type. * The `detach` method has been removed in favor of a `Drop` implementation. This notably fixes leaking thread handles on Windows. * The `Thread` structure is now appropriately annotated with `Send` and `Sync` automatically on Windows and in a custom fashion on Unix. * The unsafety of creating a thread has been pushed out to the right boundaries now. Closes #24442
Much of this code hasn't been updated in quite some time and this commit does a
small audit of the functionality:
Threadtype.detachmethod has been removed in favor of aDropimplementation. Thisnotably fixes leaking thread handles on Windows.
Threadstructure is now appropriately annotated withSendandSyncautomatically on Windows and in a custom fashion on Unix.
now.
Closes #24442