Opened 3 years ago

Closed 8 months ago

#5859 closed bug (fixed)

unsafeInterleaveIO duplicates computation when evaluated by multiple threads

Reported by: joeyadams Owned by: simonpj
Priority: normal Milestone:
Component: Core Libraries Version: 7.2.2
Keywords: Cc: pho@…, hackage.haskell.org@…, core-libraries-committee@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

When the following code is compiled with -O1 or -O2, the interleaved computation (putStrLn "eval") is performed 1000 times, rather than once:

import Control.Concurrent
import Control.Exception (evaluate)
import Control.Monad
import System.IO.Unsafe

main :: IO ()
main = do
    x <- unsafeInterleaveIO $ putStrLn "eval"
    replicateM_ 1000 $ forkIO $ evaluate x >> return ()
    threadDelay 1000000

Taking a look at the source to unsafeInterleaveIO:

{-# INLINE unsafeInterleaveIO #-}
unsafeInterleaveIO :: IO a -> IO a
unsafeInterleaveIO m = unsafeDupableInterleaveIO (noDuplicate >> m)

-- We believe that INLINE on unsafeInterleaveIO is safe, because the
-- state from this IO thread is passed explicitly to the interleaved
-- IO, so it cannot be floated out and shared.

It seems the comment about INLINE is not true. If I define the following function:

interleave :: IO a -> IO a
interleave = unsafeInterleaveIO
{-# NOINLINE interleave #-}

and replace unsafeInterleaveIO with interleave, "eval" is printed only once. If I change NOINLINE to INLINE, or if I remove the pragma altogether, "eval" is printed 1000 times.

I believe unsafeInterleaveIO should guarantee that computations are not repeated. Otherwise, we end up with strangeness like this:

import Control.Applicative
import Control.Concurrent
import Control.Monad

main :: IO ()
main = do
    chan <- newChan :: IO (Chan Int)
    mapM_ (writeChan chan) [0..999]
    items <- take 10 <$> getChanContents chan
    replicateM_ 5 $ forkIO $ putStrLn $ "items = " ++ show items
    threadDelay 1000000

which prints:

items = [0,1,2,3,4,5,6,7,8,9]
items = [10,11,12,13,14,15,16,17,18,19]
items = [20,21,22,23,24,25,26,27,28,29]
items = [30,31,32,33,34,35,36,37,38,39]
items = [40,41,42,43,44,45,46,47,48,49]

For the time being, programs can work around this by using a NOINLINE wrapper:

getChanContents' :: Chan a -> IO [a]
getChanContents' = getChanContents
{-# NOINLINE getChanContents' #-}

I tested this on Linux 64-bit with GHC 7.2.2 and ghc-7.4.0.20120111, and on Windows 32-bit with GHC 7.0.3 and 7.2.2. All of these platforms and versions exhibit the same behavior. The bug goes away when the program is compiled with -O0, or when functions returning interleaved computations are marked NOINLINE (e.g. getChanContents').

Attachments (2)

good.simpl (35.4 KB) - added by simonpj 3 years ago.
bad.simpl (43.1 KB) - added by simonpj 3 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by PHO

  • Cc pho@… added

comment:2 Changed 3 years ago by simonmar

  • difficulty set to Unknown
  • Milestone set to 7.4.2
  • Priority changed from normal to high

Thanks for the report.

comment:3 Changed 3 years ago by simonpj

This does indeed look bogus. I've done a little investigation, which I'm going to record here so I don't forget it.

Fundamentally the problem is the "state hack". (Compile with -fno-state-hack and the problem goes away.) Consider

   let x = factorial 1000 in replicate 10 (print x)

After a bit of inlining we get something like

   let x = factorial 1000 in replicate 10 (\s. wprint x s)

where s :: State# RealWorld is a state token. Now the float-in pass pushes the let-binding inwards, to give

   replicate 10 (\s.  wprint (factorial 1000) s)

and we are dead: we evaluate (factorial 1000) each time round the loop.

Why does float-in do this? Because the state hack says that the type of s means it is only applied once, which is patently false.

I think the solution is the narrow (once more) the scope of the state hack, so that it's only used in arity expansion. I know how to do this but have to set up nofib runs to compare before and after.

Thanks for the report. Hacks usually bite you in the end.

Simon

comment:4 Changed 3 years ago by simonmar

  • Owner set to simonpj

comment:5 Changed 3 years ago by simonpj

I tried disabling the use of the state hack (isStateHackType, via isOneShotBndr) in

    simplCore/Simplify.lhs 
    simplCore/OccurAnal.lhs 
    simplCore/FloatIn.lhs

(the only other state hack uses are in coreSyn/CoreArity.lhs).

This resulted in perf/compiler/parsing001 failing slightly, and perf/should_run/T5536 failing a lot. I looked into the latter. The attached program has

     484268840 bytes allocated with the full state hack
    2085048512 bytes allocated with the state hack modified as above

Replacing either of the "if" expressions in the last few lines with just its "else" branch fixes the problem.

I've also attached the -ddump-simpl output.

Changed 3 years ago by simonpj

Changed 3 years ago by simonpj

comment:6 follow-up: Changed 3 years ago by simonpj

Cf #5943. Since unsafeDupableInterleaveIO is now NONINLINE, the symptoms of this ticket might have gone away.

comment:7 Changed 3 years ago by igloo

  • Milestone changed from 7.4.2 to 7.4.3

comment:8 Changed 3 years ago by igloo

  • Milestone changed from 7.4.3 to 7.6.2

comment:9 Changed 2 years ago by liyang

  • Cc hackage.haskell.org@… added

comment:10 Changed 23 months ago by bgamari

Indeed I can't reproduce the buggy behavior with 7.6.2 and 7.7 HEAD.

Last edited 23 months ago by bgamari (previous) (diff)

comment:11 Changed 13 months ago by thoughtpolice

  • Priority changed from high to normal

Lowering priority (these tickets are assigned to older versions, so they're getting bumped as they've been around for a while).

comment:12 Changed 13 months ago by thoughtpolice

  • Milestone changed from 7.6.2 to 7.10.1

Moving to 7.10.1.

comment:13 Changed 10 months ago by thoughtpolice

  • Component changed from libraries/base to Core Libraries

Moving over to new owning component 'Core Libraries'.

comment:14 in reply to: ↑ 6 Changed 8 months ago by thomie

  • Cc core-libraries-committee@… added
  • Milestone 7.10.1 deleted
  • Resolution set to fixed
  • Status changed from new to closed

Replying to simonpj:

Cf #5943. Since unsafeDupableInterleaveIO is now NONINLINE, the symptoms of this ticket might have gone away.

I confirm, as did bgamari:

Indeed I can't reproduce the buggy behavior with 7.6.2 and 7.7 HEAD.

For reference, in commit bbcf397765953798b6d730c2bd1088c3463f3f5b:

Author: Simon Marlow <>
Date:   Fri Mar 23 12:28:18 2012 +0000

    change unsafeDupableInterleaveIO from INLINE to NOINLINE (#5943)
    
    See the comment for details.
Note: See TracTickets for help on using tickets.