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

fix freeze on @threads loop exit #45899

Merged
merged 1 commit into from
Jul 7, 2022
Merged

fix freeze on @threads loop exit #45899

merged 1 commit into from
Jul 7, 2022

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jul 1, 2022

Closes #45626, hopefully. I don't have the hardware available to reproduce this, but I think this fence was missing.

It also perhaps might be considered a defect in libuv, since it explicitly does a "cheap read" here:
https://github.com/libuv/libuv/blob/master/src/unix/async.c#L65
https://github.com/libuv/libuv/blob/master/src/win/async.c#L58
But that might not agree with the doc's claim that "If uv_async_send() is called again after the callback was called, it will be called again", if we think that means there should exist an implied happens-before edge. But the wording is imprecise as to how it defines "after the callback" on the other thread. The fast-read optimization is never required to observe the other thread's behavior, so it is never required to call the callback again, unless the user of libuv inserts their own synchronization fences that enforce progress.
http://docs.libuv.org/en/v1.x/async.html#c.uv_async_send

@vtjnash vtjnash requested a review from tkf July 1, 2022 16:37
@vchuravy vchuravy added the backport 1.8 Change should be backported to release-1.8 label Jul 4, 2022
src/partr.c Outdated Show resolved Hide resolved
src/jl_uv.c Outdated Show resolved Hide resolved
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Jul 6, 2022
@tkf tkf merged commit f7e0c7e into master Jul 7, 2022
@tkf tkf deleted the jn/45626 branch July 7, 2022 10:29
KristofferC pushed a commit that referenced this pull request Jul 7, 2022
Closes #45626, hopefully.

(cherry picked from commit f7e0c7e)
KristofferC pushed a commit that referenced this pull request Jul 8, 2022
Closes #45626, hopefully.

(cherry picked from commit f7e0c7e)
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Jul 8, 2022
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jul 9, 2022
ffucci pushed a commit to ffucci/julia that referenced this pull request Aug 11, 2022
pcjentsch pushed a commit to pcjentsch/julia that referenced this pull request Aug 18, 2022
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.

Darwin/ARM64: Julia freezes on @threads loops
5 participants