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

Mutex is not thread-safe on ARM processors #13642

Closed
jgaskins opened this issue Jul 8, 2023 · 4 comments
Closed

Mutex is not thread-safe on ARM processors #13642

jgaskins opened this issue Jul 8, 2023 · 4 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:arm status:duplicate topic:multithreading topic:stdlib:concurrency

Comments

@jgaskins
Copy link
Contributor

jgaskins commented Jul 8, 2023

The following code generates a 1M-element array across multiple fibers and then prints its size, so the output should be 1000000:

mutex = Mutex.new
numbers = Array(Int32).new(initial_capacity: 1_000_000)
done = Channel(Nil).new
concurrency = 20
iterations = 1_000_000 // concurrency
concurrency.times do
  spawn do
    iterations.times { mutex.synchronize { numbers << 0 } }
  ensure
    done.send nil
  end
end

# Wait for all fibers to complete
concurrency.times { done.receive }
sleep 100.milliseconds # Wait just a bit longer to be sure the discrepancy isn't due to a *different* race condition
pp numbers.size

On Intel processors on both macOS and Linux, it gives the correct output reliably. On ARM processors on both macOS and Linux, it comes up anywhere from a few dozen to a few thousand short.

If I use Crystal::SpinLock#sync instead of Mutex#synchronize it displays the correct output, so it seems it's something in between Mutex and SpinLock. This is good news because it means that Channel should still be thread-safe on ARM since it uses SpinLock directly.

@jgaskins jgaskins added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jul 8, 2023
@yxhuvud
Copy link
Contributor

yxhuvud commented Jul 8, 2023

Just to check, is this with or without -Dpreview_mt ?

@jgaskins
Copy link
Contributor Author

jgaskins commented Jul 8, 2023

With.

@straight-shoota
Copy link
Member

Probably related to/duplicate of #13055

@jgaskins
Copy link
Contributor Author

jgaskins commented Jul 9, 2023

@straight-shoota Yep, even the repro is almost the same. 👍🏼

@jgaskins jgaskins closed this as completed Jul 9, 2023
@Blacksmoke16 Blacksmoke16 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:arm status:duplicate topic:multithreading topic:stdlib:concurrency
Projects
None yet
Development

No branches or pull requests

4 participants