search

ronisbr / PrettyTables.jl

Print data in formatted tables.
MIT License
404 stars 38 forks source link

Redirect `stdout` to `devnull` during precompilation #231

Closed bauglir closed 10 months ago

bauglir commented 10 months ago

Calling redirect_stdout without any arguments redirects the output to a newly created Pipe^1 (which is mostly a wrapper around standard *nix pipes). Not reading data from this Pipe can result in precompilation to hang, e.g. in CI environments such as GitHub Actions (as was the case for me) or as reported here^2.

This is due to a fundamental expectation that a (*nix) kernel has on pipes, as documented^3, "write will block until sufficient data has been read from the pipe to allow the write to complete". For this reason never reading from new_stdout may cause the precompilation to hang. Explicitly redirecting to devnull circumvents this expectation and ensures precompilation succeeds.

I was able to verify the bug and the fix due to our (internal) CI systems getting stuck and trying with and without the fix (a couple of times to ensure the test was reliable/reproducible). Unfortunately, I don't have any publicly available logs to show as I couldn't get this reproduced on standard GitHub Actions runners. Likely because these always start with a fresh Julia depot and it appears that every time this becomes an issue some problem with the depot seems to trigger it.

Thanks to @vtjnash for pointing me in the right direction.

KristofferC commented 10 months ago

What's the reason for redirecting stdout at all intead of just passing an IO to the functions below?

ronisbr commented 10 months ago

Hi @bauglir !

Thanks for the very detailed report!

What's the reason for redirecting stdout at all intead of just passing an IO to the functions below?

Hi @KristofferC!

When I created the precompile statements using SnoopCompile.jl sometime ago, the time to print the first table was significantly reduced if I used those calls, which set io = stdout. I guessed that, by using IOBuffer, the system was compiling different versions of the functions compared to what is called in the REPL, when the IO is the stdout. Hence, I kept the same behavior even when I switched to PrecompilationTools.jl. However, in this case, I needed to redirect the stdout so that the user does not see a lot of output when the package gets precompiled.

Do you think that this behavior was improved in recent Julia versions and the approach deserves revisiting?

bauglir commented 10 months ago

Thanks for merging and getting the release out!