Avoid race condition in hDuplicateTo

Closed Moritz Kiefer requested to merge cocreature/ghc:fix-hduplicateto into master

In our codebase we have some code along the lines of

newStdout <- hDuplicate stdout
stderr `hDuplicateTo` stdout

to avoid stray putStrLns from corrupting a protocol (LSP) that is run over stdout.

On CI we have seen a bunch of issues where dup2 returned EBUSY so this fails with ResourceExhausted in Haskell.

I’ve spent some time looking at the docs for dup2 and the code in base and afaict the following race condition is being triggered here:

  1. The user calls hDuplicateTo stderr stdout.
  2. hDuplicateTo calls hClose_help stdout_, this closes the file handle for stdout.
  3. The file handle for stdout is now free, so another thread allocating a file might get stdout.
  4. If dup2 is called while stdout (now pointing to something else) is half-open, it returns EBUSY.

I think there might actually be an even worse case where dup2 is run after FD 1 is fully open again. In that case, you will end up not just redirecting the original stdout to stderr but also the whatever resulted in that file handle being allocated.

As far as I can tell, dup2 takes care of closing the file handle itself so there is no reason to do this in hDuplicateTo. So this PR replaces the call to hClose_help by the only part of hClose_help that we actually care about, namely, flushWriteBuffer.

I tested this on our codebase fairly extensively and haven’t been able to reproduce the issue with this patch.

I’ve looked through the commit history but the commit that introduced it 7b067f2d is over 10 years old and doesn’t have any information on why the hClose call was added originally.

Merge request reports