Opened 3 years ago

Closed 5 months ago

#10731 closed bug (fixed)

System.IO.openTempFile is not thread safe on Windows

Reported by: NeilMitchell Owned by:
Priority: normal Milestone: 8.4.1
Component: libraries/base Version: 7.10.1
Keywords: Cc: ndmitchell@…, Phyx-
Operating System: Windows Architecture: Unknown/Multiple
Type of failure: Runtime crash Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D4278
Wiki Page:

Description

Given the test program:

import Control.Concurrent
import System.IO
import Control.Monad
import System.Directory

main = do
    putStrLn "Starting"
    var <- newEmptyMVar
    tdir <- getTemporaryDirectory
    xs <- replicateM 10 $ do
        var <- newEmptyMVar
        flip forkFinally (\s -> do print s; putMVar var ()) $ do
            replicateM_ 100000 $ do
                (file, h) <- openTempFile tdir "test.txt"
                hClose h
                removeFile file
        return var
    mapM_ takeMVar xs

If I compile and run that with ghc --make TmpFile.hs -threaded && tmpfile +RTS -N5 I get:

Starting
Left C:\Users\NDMIT_~1\AppData\Local\Temp\: openTempFile: permission denied (Permission denied)
Left C:\Users\NDMIT_~1\AppData\Local\Temp\: openTempFile: permission denied (Permission denied)
...

I've reproduced this on GHC 7.8 and 7.10.1. We hit this in production about twice a day.

Change History (7)

comment:1 Changed 3 years ago by NeilMitchell

Note that the exceptions start happening within seconds - this doesn't seem to be a difficult race condition to hit. I'm on Windows 8.1, but it reproduces just as easily with Windows 7.

comment:2 Changed 3 years ago by NeilMitchell

I've avoided this issue in the extra package (https://github.com/ndmitchell/extra/blob/master/src/System/IO/Extra.hs) with three techniques:

  • To avoid races within a process, I have a single IORef which I increment to find unique names. That guarantees that a process will never clash with itself.
  • To help avoid cross-process races I start that IORef at a random starting point (using the start time plus Process Id would be ideal, but I couldn't get that easily, so I use the time).
  • If all else fails, I retry up to 5 times on IO errors.

With those features, I've yet to get a failure.

comment:3 Changed 15 months ago by rwbarton

Cc: Phyx- added

Phyx-, do you know of a threadsafe way to create and open a temporary file on Windows?

comment:4 Changed 15 months ago by Phyx-

GetTempFileName with uUnique 0 should be thread safe. The call will do most of the steps described in comment:2.

comment:5 Changed 5 months ago by Phyx-

Differential Rev(s): Phab:D4278
Status: newpatch

comment:6 Changed 5 months ago by Ben Gamari <ben@…>

In 46287af0/ghc:

Make System.IO.openTempFile thread-safe on Windows

This calls out to the Win32 API `GetTempFileName` to generate
a temporary file. Using `uUnique = 0` guarantees that the file
we get back is unique and the file is "reserved" by creating it.

Test Plan:
./validate

I can't think of any sensible tests that shouldn't run for a while
to verify. So the example in #10731 was ran for a while and no
collisions in new code

Reviewers: hvr, bgamari, erikd

Reviewed By: bgamari

Subscribers: RyanGlScott, rwbarton, thomie, carter

GHC Trac Issues: #10731

Differential Revision: https://phabricator.haskell.org/D4278

comment:7 Changed 5 months ago by bgamari

Milestone: 8.4.1
Resolution: fixed
Status: patchclosed
Note: See TracTickets for help on using tickets.