fix IO deadlock condition #22886
fix IO deadlock condition #22886
Conversation
src/jl_uv.c
Outdated
| // attempt graceful shutdown of writable streams to give them a chance to flush first | ||
| // TODO: introduce a uv_drain cb API instead instead of abusing uv_shutdown in this way |
test/spawn.jl
Outdated
| sleep(0.5) # give cat a chance to fill the write buffer for stdout | ||
| close(stdout.in) # make sure we can still close the write end | ||
| @test sizeof(readstring(stdout)) == 1048576 * 2 # make sure we get all the data | ||
| success(p) |
when calling uv_shutdown on a handle being written to by another process we might never get the UV__POLLOUT notification but we also don't need to delay the uv_close call if we have nothing to write in the future, we should introduce a new `uv_drain` callback, instead of continuing to abuse uv_shutdown for this purpose fix #22832
| @@ -469,3 +479,19 @@ let c = `ls -l "foo bar"` | |||
| @test length(c) == 3 | |||
| @test eachindex(c) == 1:3 | |||
| end | |||
|
|
|||
| ## Deadlock in spawning a cmd (#22832) | |||
| # FIXME? | |||
There was a problem hiding this comment.
what was broken about this? use test_broken
There was a problem hiding this comment.
hello?
PLEASE communicate more here. No one can read your mind. What didn't work about this, a FIXME comment with no details and a commented out test is better than nothing, but doesn't actually help anyone figure out what is wrong here and why the test doesn't work, can't use @test_broken, or the fix is incomplete. You do this over and over and over again and I implore you every time to cut it out.
There was a problem hiding this comment.
Mentioned in the merge commit text. I don't know the answers to any of those questions, so I can't provide more information. If I knew any of those answers, I would have already fixed the test.
There was a problem hiding this comment.
Would test_broken have worked? A simple response to the question "it froze on CI and couldn't reproduce locally" would be a lot more visible than the merge commit text.
…n't dead lock. Ugly workaround." This reverts commit 2a5c158. Issue JuliaLang/julia#22832 is fixed with JuliaLang/julia#22886
…n't dead lock. Ugly workaround." This reverts commit 2a5c158. Issue JuliaLang/julia#22832 is fixed with JuliaLang/julia#22886
so it doesn't get used on versions of Julia susceptible to JuliaLang/julia#22886
so it doesn't get used on versions of Julia susceptible to JuliaLang/julia#22886
…n't dead lock. Ugly workaround." This reverts commit 2a5c158. Issue JuliaLang/julia#22832 is fixed with JuliaLang/julia#22886
when calling uv_shutdown on a handle being written to by another process we might never get the UV__POLLOUT notification but we also don't need to delay the uv_close call if we have nothing to write in the future, we should introduce a new `uv_drain` callback, instead of continuing to abuse uv_shutdown for this purpose fix #22832 Ref #22886 (cherry picked from commit 8949a95)
when calling uv_shutdown on a handle being written to by another process
we might never get the UV__POLLOUT notification
but we also don't need to delay the uv_close call
if we have nothing to write
in the future, we should introduce a new
uv_draincallback,instead of continuing to abuse uv_shutdown for this purpose
fix #22832