Skip to content

Std/thread: deny unsafe op in unsafe fn#74225

Merged
bors merged 5 commits into
rust-lang:masterfrom
poliorcetics:std-thread-unsafe-op-in-unsafe-fn
Sep 26, 2020
Merged

Std/thread: deny unsafe op in unsafe fn#74225
bors merged 5 commits into
rust-lang:masterfrom
poliorcetics:std-thread-unsafe-op-in-unsafe-fn

Conversation

@poliorcetics

@poliorcetics poliorcetics commented Jul 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Partial fix of #73904.

This encloses unsafe operations in unsafe fn in libstd/thread.
@rustbot modify labels: F-unsafe-block-in-unsafe-fn

@rust-highfive

rust-highfive commented Jul 10, 2020

Copy link
Copy Markdown
Contributor

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 10, 2020
@LeSeulArtichaut LeSeulArtichaut mentioned this pull request Jul 11, 2020
Closed
22 tasks
@bors

bors commented Jul 28, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #73265) made this pull request unmergeable. Please resolve the merge conflicts.

@poliorcetics poliorcetics force-pushed the std-thread-unsafe-op-in-unsafe-fn branch from dfa69f8 to 58c7e35 Compare July 28, 2020 16:55
@bors

bors commented Aug 10, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #75351) made this pull request unmergeable. Please resolve the merge conflicts.

@poliorcetics poliorcetics force-pushed the std-thread-unsafe-op-in-unsafe-fn branch from 58c7e35 to 3a22b21 Compare August 10, 2020 17:00
@crlf0710 crlf0710 added T-libs Relevant to the library team, which will review and decide on the PR/issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 28, 2020
LeSeulArtichaut
LeSeulArtichaut reviewed Aug 28, 2020
View reviewed changes

@LeSeulArtichaut LeSeulArtichaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a quick high-level review (I can't go any deeper because I know pretty much nothing about threads and TLS and OSes). Thanks again for helping with this @poliorcetics!

Comment thread library/std/src/thread/local.rs
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
@poliorcetics

poliorcetics commented Aug 29, 2020

Copy link
Copy Markdown
Contributor Author

In case GitHub makes it non-obvious: I've left answers to some of your suggestions and modified the other comments in response.

Mark-Simulacrum
Mark-Simulacrum reviewed Sep 15, 2020
View reviewed changes
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
Comment thread library/std/src/thread/local.rs Outdated
@poliorcetics

poliorcetics commented Sep 20, 2020

Copy link
Copy Markdown
Contributor Author

I modified all the comments that were noted. There are some part of those that could maybe go as documentation for their enclosing method and not inner SAFETY comments but, apart from those I did, I'm not sure how to write them.

@joshtriplett

joshtriplett commented Sep 20, 2020

Copy link
Copy Markdown
Member

@bors r+ rollup

@bors

bors commented Sep 20, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 0e56b52 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#74225 - poliorcetics:std-thread-unsafe-op-i…
4cb73e3 
…n-unsafe-fn, r=joshtriplett

Std/thread: deny unsafe op in unsafe fn

Partial fix of rust-lang#73904.

This encloses `unsafe` operations in `unsafe fn` in `libstd/thread`.
@rustbot modify labels: F-unsafe-block-in-unsafe-fn
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Sep 21, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 21, 2020
@Dylan-DPC
Rollup merge of rust-lang#74225 - poliorcetics:std-thread-unsafe-op-i…
7e86f01 
…n-unsafe-fn, r=joshtriplett

Std/thread: deny unsafe op in unsafe fn

Partial fix of rust-lang#73904.

This encloses `unsafe` operations in `unsafe fn` in `libstd/thread`.
@rustbot modify labels: F-unsafe-block-in-unsafe-fn
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Sep 21, 2020
Closed
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 21, 2020
@RalfJung
Rollup merge of rust-lang#74225 - poliorcetics:std-thread-unsafe-op-i…
be347bc 
…n-unsafe-fn, r=joshtriplett

Std/thread: deny unsafe op in unsafe fn

Partial fix of rust-lang#73904.

This encloses `unsafe` operations in `unsafe fn` in `libstd/thread`.
@rustbot modify labels: F-unsafe-block-in-unsafe-fn
@RalfJung RalfJung mentioned this pull request Sep 21, 2020
Closed
@RalfJung

RalfJung commented Sep 21, 2020

Copy link
Copy Markdown
Member

Failed in #77001 (comment)
@bors r- rollup=iffy

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 21, 2020
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 21, 2020
@poliorcetics poliorcetics force-pushed the std-thread-unsafe-op-in-unsafe-fn branch from 0e56b52 to 3afadaa Compare September 21, 2020 20:38
@poliorcetics

poliorcetics commented Sep 21, 2020

Copy link
Copy Markdown
Contributor Author

I fixed the bug for wasm32.

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 21, 2020
@joshtriplett

joshtriplett commented Sep 24, 2020

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Sep 24, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit d01bd19 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 24, 2020
@bors

bors commented Sep 26, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit d01bd19 with merge 9e1c436...

@bors

bors commented Sep 26, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions, checks-azure
Approved by: joshtriplett
Pushing 9e1c436 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 26, 2020
@bors bors merged commit 9e1c436 into rust-lang:master Sep 26, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 26, 2020
@poliorcetics poliorcetics deleted the std-thread-unsafe-op-in-unsafe-fn branch September 27, 2020 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

+1 more reviewer

@LeSeulArtichaut LeSeulArtichaut LeSeulArtichaut left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@joshtriplett joshtriplett

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.48.0

Development

Successfully merging this pull request may close these issues.

9 participants

@poliorcetics @rust-highfive @bors @joshtriplett @RalfJung @Mark-Simulacrum @LeSeulArtichaut @crlf0710 @rustbot