Skip to content
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

channel: Do not call current_thread_id inside iteration #802

Merged
merged 1 commit into from Mar 18, 2022
Merged

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Mar 17, 2022

This should result in the same behavior as before 10009bd.

This works around an upstream bug related to TLS access on aarch64 linux, which was reported in firefox.

@glandium
Copy link

glandium commented Mar 17, 2022

when LTO is enabled,

FWIW, since my comment about that, I actually confirmed that LTO is not involved. It might be dependent on the rust version, though (not fully confirmed yet).

Edit: it looks like it's not version-dependent.

@glandium
Copy link

glandium commented Mar 18, 2022

FWIW, something else that works is changing the current_thread_id function to not have a thread-local cache.

Edit: another thing that works is annotating current_thread_id with #[inline(never)] so it would seem something might be going wrong with the inlining of the thread-local.

@taiki-e
Copy link
Member Author

taiki-e commented Mar 18, 2022

@glandium Thanks for investigating this!
If there is a problem with the inlining of TLS access, it may work fine on rustc before rust-lang/rust@641d3b0 (1.54) and if LTO is disabled.

bors r+

@taiki-e
Copy link
Member Author

taiki-e commented Mar 18, 2022

(Possibly related: rust-lang/rust#48967)

@bors
Copy link
Contributor

bors bot commented Mar 18, 2022

Build succeeded:

@bors bors bot merged commit eb6cb05 into master Mar 18, 2022
@bors bors bot deleted the try-select branch March 18, 2022 15:04
bors bot added a commit that referenced this pull request Mar 18, 2022
805: Prepare for the next release r=taiki-e a=taiki-e

- crossbeam-channel 0.5.3 -> 0.5.4
  - Workaround a bug in upstream related to TLS access on AArch64 Linux. (#802)

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@taiki-e
Copy link
Member Author

taiki-e commented Mar 18, 2022

Published in crossbeam-channel 0.5.4.

UPDATE: I've yanked affected releases.

pld-gitsync pushed a commit to pld-linux/firefox that referenced this pull request Mar 19, 2022
tatsuya6502 added a commit to moka-rs/moka that referenced this pull request Mar 20, 2022
crossbeam-channel v0.5.4 has a workaround for a bug in upstream (rustc or LLVM?)
related to TLS access on AArch64 Linux:
crossbeam-rs/crossbeam#802
@glandium
Copy link

For the record, taking 0.5.4 works, and changing try_select to have #[inline(never)] breaks it again.

@klensy klensy mentioned this pull request Apr 18, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 27, 2022
…crum

Bump deps

Update few deps:

First commit: vulnerable or yanked ones:
* openssl-src 111.17.0+1.1.1m -> 111.18.0+1.1.1n vuln https://rustsec.org/advisories/RUSTSEC-2022-0014
* crossbeam-channel 0.5.2 -> 0.5.4 yanked: crossbeam-rs/crossbeam#802 (https://github.com/crossbeam-rs/crossbeam/blob/crossbeam-channel-0.5.4/crossbeam-channel/CHANGELOG.md)
* crossbeam-utils 0.8.6 -> 0.8.8 yanked: GHSA-qc84-gqf4-9926 (https://github.com/crossbeam-rs/crossbeam/blob/crossbeam-utils-0.8.8/crossbeam-utils/CHANGELOG.md)

Second commit: no notable changes, most of them touched only to remove other ones:
* Updating ammonia v3.1.3 -> v3.2.0
* Updating html5ever v0.25.1 -> v0.26.0
* Updating markup5ever v0.10.1 -> v0.11.0
* Removing markup5ever_rcdom v0.1.0
* Updating phf v0.8.0 -> v0.10.1
* Updating phf_codegen v0.8.0 -> v0.10.0
* Updating phf_generator v0.8.0 -> v0.10.0
* Updating phf_shared v0.8.0 -> v0.10.0
* Updating rand v0.8.4 -> v0.8.5
* Removing rand_hc v0.3.0
* Removing rand_pcg v0.2.1
* Updating string_cache v0.8.0 -> v0.8.3
* Updating string_cache_codegen v0.5.1 -> v0.5.2
* Removing xml5ever v0.16.1

drops markup5ever_rcdom, rand_hc, rand_pcg, xml5ever versions

* rand 0.8.4 -> 0.8.5 (https://github.com/rust-random/rand/blob/0.8.5/CHANGELOG.md#085---2021-08-20)

Third one is perf oriented:
* proc-macro2 v1.0.30 -> v1.0.37 dtolnay/proc-macro2@1.0.30...1.0.37 (https://github.com/dtolnay/proc-macro2/releases, for example https://github.com/dtolnay/proc-macro2/releases/tag/1.0.36)
* quote v1.0.7 -> v1.0.18 dtolnay/quote@1.0.7...1.0.18 (https://github.com/dtolnay/quote/releases) multiple perf improvements: https://github.com/dtolnay/quote/releases/tag/1.0.16, https://github.com/dtolnay/quote/releases/tag/1.0.14, https://github.com/dtolnay/quote/releases/tag/1.0.11
* syn v1.0.80 -> v1.0.91 dtolnay/syn@1.0.80...1.0.91 (https://github.com/dtolnay/syn/releases): didn't find good examples, but given, that there exist private api across `proc-macro2`, `quote` by the same author, *i think* it may take advantage of it.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2023
…ulacrum

Avoid tls access while iterating through mpsc thread entries

Upstream fix: crossbeam-rs/crossbeam#802. Possibly fixes rust-lang#113726.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jul 22, 2023
Avoid tls access while iterating through mpsc thread entries

Upstream fix: crossbeam-rs/crossbeam#802. Possibly fixes rust-lang/rust#113726.
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Oct 17, 2023
Avoid tls access while iterating through mpsc thread entries

Upstream fix: crossbeam-rs/crossbeam#802. Possibly fixes rust-lang/rust#113726.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants