Skip to content

Make CoreLatch::set use SeqCst instead of AcqRel#1297

Merged
cuviper merged 1 commit intorayon-rs:mainfrom
cuviper:set-seq-cst
Apr 21, 2026
Merged

Make CoreLatch::set use SeqCst instead of AcqRel#1297
cuviper merged 1 commit intorayon-rs:mainfrom
cuviper:set-seq-cst

Conversation

@cuviper
Copy link
Copy Markdown
Member

@cuviper cuviper commented Apr 21, 2026

Every other modification of this variable uses SeqCst, which is justified in the sleep README. This particular choice of AcqRel was not discussed during its addition in #746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency.

The only other ordering difference is CoreLatch::probe's load with Acquire, which should be fine because this doesn't need consistency with the sleep counters.

Assisted-by: Claude Code

@cuviper
Copy link
Copy Markdown
Member Author

cuviper commented Apr 21, 2026

cc @nikomatsakis in case you have any dusty memories about this.

I should also look into using some aarch64 runners in CI, since that has weaker memory ordering than x86, which could make a difference here. But I think it would be quite a rare race for this one to really cause any problem. (edit: done in #1298)

Every other modification of this variable uses `SeqCst`, which is
justified in the sleep README. This particular choice of `AcqRel` was
not discussed during its addition in rayon-rs#746, nor rayon-rs/rfcs#5, so I
suspect was simply an oversight from earlier development. We probably do
want this to participate in the same sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with
`Acquire`, which should be fine because this doesn't need consistency
with the sleep counters.

Assisted-by: Claude Code
@cuviper cuviper added this pull request to the merge queue Apr 21, 2026
Merged via the queue into rayon-rs:main with commit 101822b Apr 21, 2026
4 checks passed
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Apr 22, 2026
…Rel`

Every other modification of this variable uses `SeqCst`, which is
justified in the sleep README. This particular choice of `AcqRel` was
not discussed during its addition in rayon-rs/rayon#746, nor
rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier
development. We probably do want this to participate in the same
sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with
`Acquire`, which should be fine because this doesn't need consistency
with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice
this would be quite rare to cause any problems, but it *could* be a
source of non-deterministic bugs on targets with weak memory ordering.
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Apr 22, 2026
rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel`

Every other modification of this variable uses `SeqCst`, which is justified in the sleep README. This particular choice of `AcqRel` was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with `Acquire`, which should be fine because this doesn't need consistency with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it *could* be a source of non-deterministic bugs on targets with weak memory ordering.
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Apr 23, 2026
rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel`

Every other modification of this variable uses `SeqCst`, which is justified in the sleep README. This particular choice of `AcqRel` was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with `Acquire`, which should be fine because this doesn't need consistency with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it *could* be a source of non-deterministic bugs on targets with weak memory ordering.
github-actions Bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Apr 24, 2026
rustc_thread_pool: Make `CoreLatch::set` use `SeqCst` instead of `AcqRel`

Every other modification of this variable uses `SeqCst`, which is justified in the sleep README. This particular choice of `AcqRel` was not discussed during its addition in rayon-rs/rayon#746, nor rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier development. We probably do want this to participate in the same sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with `Acquire`, which should be fine because this doesn't need consistency with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice this would be quite rare to cause any problems, but it *could* be a source of non-deterministic bugs on targets with weak memory ordering.
Bryanskiy pushed a commit to Bryanskiy/rust that referenced this pull request May 4, 2026
…Rel`

Every other modification of this variable uses `SeqCst`, which is
justified in the sleep README. This particular choice of `AcqRel` was
not discussed during its addition in rayon-rs/rayon#746, nor
rayon-rs/rfcs#5, so I suspect was simply an oversight from earlier
development. We probably do want this to participate in the same
sequential consistency.

The only other ordering difference is `CoreLatch::probe`'s load with
`Acquire`, which should be fine because this doesn't need consistency
with the sleep counters.

See also rayon-rs/rayon#1297. As I commented there, I think in practice
this would be quite rare to cause any problems, but it *could* be a
source of non-deterministic bugs on targets with weak memory ordering.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant