Misleading MVar documentation

Merry Christmas all! Is it just me, or does the Control.Concurrent.MVar documentation seem a bit misleading? In particular, we should explicitly note the race conditions for not just swapMVar but also readMVar, withMVar, modifyMVar_ and modifyMVar, and clarify that the safety guarantees of the latter three pertain to their handling of asynchronous exceptions. It might also be good to tell people that if they need race-free operations of this style, STM is a good alternative to look at, even if only one variable is being synchronized over. Cheers, Edward

ezyang:
Merry Christmas all!
Is it just me, or does the Control.Concurrent.MVar documentation seem a bit misleading? In particular, we should explicitly note the race conditions for not just swapMVar but also readMVar, withMVar, modifyMVar_ and modifyMVar, and clarify that the safety guarantees of the latter three pertain to their handling of asynchronous exceptions.
It might also be good to tell people that if they need race-free operations of this style, STM is a good alternative to look at, even if only one variable is being synchronized over.
That would be a good contribution.

Here is one suggested doc patch. Comments and revisions welcome: ezyang@javelin:~/Dev/ghc-clean/libraries/base/Control/Concurrent$ darcs whatsnew -u hunk ./Control/Concurrent/MVar.hs 59 {-| This is a combination of 'takeMVar' and 'putMVar'; ie. it takes the value - from the 'MVar', puts it back, and also returns it. + from the 'MVar', puts it back, and also returns it. This function + is race safe only if there are no other producers (i.e. threads calling + 'putMVar') for this 'MVar'. -} readMVar :: MVar a -> IO a readMVar m = hunk ./Control/Concurrent/MVar.hs 72 {-| Take a value from an 'MVar', put a new value into the 'MVar' and - return the value taken. Note that there is a race condition whereby - another process can put something in the 'MVar' after the take - happens but before the put does. + return the value taken. This function is race safe only if there are + no other prodcuers for this 'MVar'. -} swapMVar :: MVar a -> a -> IO a swapMVar mvar new = hunk ./Control/Concurrent/MVar.hs 83 return old {-| - 'withMVar' is a safe wrapper for operating on the contents of an - 'MVar'. This operation is exception-safe: it will replace the + 'withMVar' is an exception-safe wrapper for operating on the contents + of an 'MVar'. This operation is exception-safe: it will replace the original contents of the 'MVar' if an exception is raised (see hunk ./Control/Concurrent/MVar.hs 86 - "Control.Exception"). + "Control.Exception"). However, it is only race safe if there are no + other producers for this 'MVar'. -} {-# INLINE withMVar #-} -- inlining has been reported to have dramatic effects; see hunk ./Control/Concurrent/MVar.hs 101 return b {-| - A safe wrapper for modifying the contents of an 'MVar'. Like 'withMVar', $ - 'modifyMVar' will replace the original contents of the 'MVar' if an - exception is raised during the operation. + An exception-safe wrapper for modifying the contents of an 'MVar'. + Like 'withMVar', 'modifyMVar' will replace the original contents of + the 'MVar' if an exception is raised during the operation. This + function is only race safe if there are no other producers for this + 'MVar'. -} {-# INLINE modifyMVar_ #-} modifyMVar_ :: MVar a -> (a -> IO a) -> IO () Cheers, Edward

On Fri, Dec 24, 2010 at 5:32 PM, Edward Z. Yang
Merry Christmas all!
Is it just me, or does the Control.Concurrent.MVar documentation seem a bit misleading? In particular, we should explicitly note the race conditions for not just swapMVar but also readMVar, withMVar, modifyMVar_ and modifyMVar, and clarify that the safety guarantees of the latter three pertain to their handling of asynchronous exceptions.
It might also be good to tell people that if they need race-free operations of this style, STM is a good alternative to look at, even if only one variable is being synchronized over.
This reminds me, I recall someone showing me some runtimes that implied for nearly all programs TVars had better performance than MVars. I can't find those results on google. I did find this thread: http://www.mail-archive.com/haskell-cafe@haskell.org/msg50734.html The links in Don's mail are broken. It seems that Simon Marlow's paper directory didn't survive the server transition: http://www.haskell.org/~simonmar/papers/ Jason

Wasn't that result John Launchbury's? I recalled hearing something similar as well. Cheers, Edward

Hi!
On Sat, Dec 25, 2010 at 2:32 AM, Edward Z. Yang
In particular, we should explicitly note the race conditions for not just swapMVar but also readMVar, withMVar, modifyMVar_ and modifyMVar,
I am not sure if this are really race conditions? The point is that readMVar, withMVar and others do not return until they can return the value back and after that the value is the same as it was at the beginning of the call. So if somebody manages to put the value in then those functions wait that the value is removed. Race condition would mean for me that some other thread could corrupt the data. This is not the case here. So I would argue that it would be better to write that those function can block. Not that there are race-conditions. Race-conditions (for me) imply that invariants can change based on some other thread, here they hold, when/if the function returns. This is why I proposed tryReadMVar and tryModifyMVar here: http://hackage.haskell.org/trac/ghc/ticket/4535 Mitar

Excerpts from Mitar's message of Fri Dec 24 23:18:43 -0500 2010:
I am not sure if this are really race conditions? The point is that readMVar, withMVar and others do not return until they can return the value back and after that the value is the same as it was at the beginning of the call. So if somebody manages to put the value in then those functions wait that the value is removed.
Race condition would mean for me that some other thread could corrupt the data. This is not the case here.
I think you're right. A further comment is that you don't really need stringent timing conditions (which is the only thing I think of when I hear "race") to see another thread "grab" the mvar underneath you: a script as simple as this sees this behavior due to MVar's fairness properties: import Control.Concurrent.MVar import Control.Concurrent main = do r <- newMVar 1 forkIO $ do putMVar r 2 print "Executed" threadDelay 200 readMVar r print "Not executed"
So I would argue that it would be better to write that those function can block. Not that there are race-conditions. Race-conditions (for me) imply that invariants can change based on some other thread, here they hold, when/if the function returns.
Sure.
This is why I proposed tryReadMVar and tryModifyMVar here:
They seem like good suggestions. Cheers, Edward

Hi!
On Sat, Dec 25, 2010 at 11:58 AM, Edward Z. Yang
I think you're right. A further comment is that you don't really need stringent timing conditions (which is the only thing I think of when I hear "race") to see another thread "grab" the mvar underneath you
Yes, MVars are (bounded, 1 space long) queues with predictable behavior. Maybe we should change documentation for swapMVar (and others) and replace notion of race condition with that it can block. This would be also in accordance with functions I proposed. So current functions are blocking (and we document that) and we introduce non-blocking versions. Mitar

Mitar wrote:
Hi!
On Sat, Dec 25, 2010 at 11:58 AM, Edward Z. Yang
wrote: I think you're right. A further comment is that you don't really need stringent timing conditions (which is the only thing I think of when I hear "race") to see another thread "grab" the mvar underneath you
Yes, MVars are (bounded, 1 space long) queues with predictable behavior.
Maybe we should change documentation for swapMVar (and others) and replace notion of race condition with that it can block.
But this was not the issue this thread was about, namely: If readMVar (to pick one example) is applied to a full MVar with a pending putMVar, then readMVar will block *after* reading the value from the MVar, before being able to put it back. This is surprising, as it means that the value of the MVar can change during the execution of readMVar. (it will be changed back before readMVar completes but then the damage may already be done.) I think atomicity is the right concept here: All these operations consist of two separate steps (and can be interrupted in the middle) and this behaviour is observable if the MVar is not used in a single token mutex fashion, where either the MVar is full or there is exactly one thread owning the token. (Unlike simple mutexes, the MVar token can be labeled and re-labeled by the thread owning it.) Best regards, Bertram

These are interesting, opposed perspectives, and I suspect what would be good is to treat both situations. I think perhaps what would be good to put in the introduction is the conceptual model of MVars: that is, take and put are the fundamental operations, and everything else is composed of them. With additional constraints on who is writing and reading MVars, you can assume more safety properties, but you have to ensure that those are indeed held (or you should use STM instead.) I'll try another writeup. Does anyone know where the original papers for MVars might be? Cheers, Edward

On 12/01/11 15:53, Edward Z. Yang wrote:
These are interesting, opposed perspectives, and I suspect what would be good is to treat both situations. I think perhaps what would be good to put in the introduction is the conceptual model of MVars: that is, take and put are the fundamental operations, and everything else is composed of them. With additional constraints on who is writing and reading MVars, you can assume more safety properties, but you have to ensure that those are indeed held (or you should use STM instead.)
I'll try another writeup. Does anyone know where the original papers for MVars might be?
I think the original paper is "Concurrent Haskell", available here: http://www.haskell.org/ghc/docs/papers/concurrent-haskell.ps.gz and here: http://research.microsoft.com/en-us/um/people/simonpj/papers/concurrent-hask... Thanks, Neil.

On Wed, Jan 12, 2011 at 11:23 AM, Neil Brown
On 12/01/11 15:53, Edward Z. Yang wrote:
These are interesting, opposed perspectives, and I suspect what would be good is to treat both situations. I think perhaps what would be good to put in the introduction is the conceptual model of MVars: that is, take and put are the fundamental operations, and everything else is composed of them. With additional constraints on who is writing and reading MVars, you can assume more safety properties, but you have to ensure that those are indeed held (or you should use STM instead.)
I'll try another writeup. Does anyone know where the original papers for MVars might be?
I think the original paper is "Concurrent Haskell", available here:
http://www.haskell.org/ghc/docs/papers/concurrent-haskell.ps.gz
and here:
http://research.microsoft.com/en-us/um/people/simonpj/papers/concurrent-hask...
Actually, the first presentation of M-structures is rather older than that. See Barth, Nikhil, and Arvind's FPCA '91 paper: http://portal.acm.org/citation.cfm?id=652538 The original formulation was indeed in terms of "take" and "put", though unconditional read and write primitives were prtty commonly used in Id programs. The take/put view can also usefully be thought of as a 1-element blocking channel. -Jan-Willem MAessen
Thanks,
Neil.
_______________________________________________ Haskell-Cafe mailing list Haskell-Cafe@haskell.org http://www.haskell.org/mailman/listinfo/haskell-cafe

On 16 Jan 2011, at 03:58, Jan-Willem Maessen wrote:
Actually, the first presentation of M-structures is rather older than that. See Barth, Nikhil, and Arvind's FPCA '91 paper: http://portal.acm.org/citation.cfm?id=652538
The original formulation was indeed in terms of "take" and "put", though unconditional read and write primitives were prtty commonly used in Id programs. The take/put view can also usefully be thought of as a 1-element blocking channel.
The full spectrum of one-element communication protocols is set out in H R Simpson. The MASCOT method. Software Engineering Journal, 1(3):103– 120, March 1986. The notation was known as Real Time Networks. In this scheme, there are four types of protocol, each useful in different circumstances: blocking read, blocking write: a channel non-blocking read, blocking write: a constant blocking read, non-blocking write: a signal non-blocking read, non-blocking write: a pool The first of these, channel, corresponds to the MVar. The pool corresponds to an IORef. Regards, Malcolm

Ok, here is an updated doc patch. I've also added a substantial introduction section. diff -rN -u old-base/Control/Concurrent/MVar.hs new-base/Control/Concurrent/MVar.hs --- old-base/Control/Concurrent/MVar.hs 2011-01-13 16:26:59.000000000 +0000 +++ new-base/Control/Concurrent/MVar.hs 2011-01-13 16:27:00.000000000 +0000 @@ -9,7 +9,103 @@ -- Stability : experimental -- Portability : non-portable (concurrency) -- --- Synchronising variables +-- An @'MVar' t@ is mutable location that is either empty or contains a +-- value of type @t@. It has two fundamental operations: 'putMVar' +-- which fills an 'MVar' if it is empty and blocks otherwise, and +-- 'takeMVar' which empties an 'MVar' if it is full and blocks +-- otherwise. They can be used in multiple different ways: +-- +-- 1. As synchronized mutable variables, +-- 2. As channels, with 'takeMVar' and 'putMVar' as receive and send, and +-- 3. As a binary semaphore @'MVar' ()@, with 'takeMVar' and 'putMVar' as +-- wait and signal. +-- +-- They were introduced in the paper "Concurrent Haskell" by Simon +-- Peyton Jones, Andrew Gordon and Sigbjorn Finne, though some details +-- of their implementation have since then changed (in particular, a +-- put on a full MVar used to error, but now merely blocks.) +-- +-- * Applicability +-- +-- 'MVar's offer more flexibility than 'IORef's, but less flexibility +-- than 'STM'. They are appropriate for building synchronization +-- primitives and performing simple interthread communication; however +-- they are very simple and susceptible to race conditions, deadlocks or +-- uncaught exceptions. Do not use them if you need perform larger +-- atomic operations such as reading from multiple variables: use 'STM' +-- instead. +-- +-- In particular, the "bigger" functions in this module ('readMVar', +-- 'swapMVar', 'withMVar', 'modifyMVar_' and 'modifyMVar') are simply +-- compositions a 'takeMVar' followed by a 'putMVar' with exception safety. +-- These only have atomicity guarantees if all other threads +-- perform a 'takeMVar' before a 'putMVar' as well; otherwise, they may +-- block. +-- +-- * Fairness +-- +-- No process can be blocked indefinitely on an 'MVar' unless another +-- process holds that 'MVar' indefinitely. One usual implementation of +-- this fairness guarantee is that processed blocked on an 'MVar' are +-- served in a first-in-first-out fashion, but this is not guaranteed +-- in the semantics. +-- +-- * Gotchas +-- +-- Like many other Haskell data structures, 'MVar's are lazy. This +-- means that if you place an expensive unevaluated thunk inside an +-- 'MVar', it will be evaluated by the thread that consumes it, not the +-- thread that produced it. Be sure to 'evaluate' values to be placed +-- in an 'MVar' to the appropriate normal form, or utilize a strict +-- MVar provided by the strict-concurrency package. +-- +-- * Example +-- +-- Consider the following concurrent data structure, a skip channel. +-- This is a channel for an intermittent source of high bandwidth +-- information (for example, mouse movement events.) Writing to the +-- channel never blocks, and reading from the channel only returns the +-- most recent value, or blocks if there are no new values. Multiple +-- readers are supported with a @dupSkipChan@ operation. +-- +-- A skip channel is a pair of 'MVar's: the second 'MVar' is a semaphore +-- for this particular reader: it is full if there is a value in the +-- channel that this reader has not read yet, and empty otherwise. +-- +-- @ +-- data SkipChan a = SkipChan (MVar (a, [MVar ()])) (MVar ()) +-- +-- newSkipChan :: IO (SkipChan a) +-- newSkipChan = do +-- sem <- newEmptyMVar +-- main <- newMVar (undefined, [sem]) +-- return (SkipChan main sem) +-- +-- putSkipChan :: SkipChan a -> a -> IO () +-- putSkipChan (SkipChan main _) v = do +-- (_, sems) <- takeMVar main +-- putMVar main (v, []) +-- mapM_ (\sem -> putMVar sem ()) sems +-- +-- getSkipChan :: SkipChan a -> IO a +-- getSkipChan (SkipChan main sem) = do +-- takeMVar sem +-- (v, sems) <- takeMVar main +-- putMVar main (v, sem:sems) +-- return v +-- +-- dupSkipChan :: SkipChan a -> IO (SkipChan a) +-- dupSkipChan (SkipChan main _) = do +-- sem <- newEmptyMVar +-- (v, sems) <- takeMVar main +-- putMVar main (v, sem:sems) +-- return (SkipChan main sem) +-- @ +-- +-- This example was adapted from the original Concurrent Haskell paper. +-- For more examples of 'MVar's being used to build higher-level +-- synchronization primitives, see 'Control.Concurrent.Chan' and +-- 'Control.Concurrent.QSem'. -- ----------------------------------------------------------------------------- @@ -56,7 +152,9 @@ {-| This is a combination of 'takeMVar' and 'putMVar'; ie. it takes the value - from the 'MVar', puts it back, and also returns it. + from the 'MVar', puts it back, and also returns it. This function + is atomic only if there are no other producers (i.e. threads calling + 'putMVar') for this 'MVar'. -} readMVar :: MVar a -> IO a readMVar m = @@ -67,9 +165,8 @@ {-| Take a value from an 'MVar', put a new value into the 'MVar' and - return the value taken. Note that there is a race condition whereby - another process can put something in the 'MVar' after the take - happens but before the put does. + return the value taken. This function is atomic only if there are + no other producers for this 'MVar'. -} swapMVar :: MVar a -> a -> IO a swapMVar mvar new = @@ -79,10 +176,11 @@ return old {-| - 'withMVar' is a safe wrapper for operating on the contents of an - 'MVar'. This operation is exception-safe: it will replace the + 'withMVar' is an exception-safe wrapper for operating on the contents + of an 'MVar'. This operation is exception-safe: it will replace the original contents of the 'MVar' if an exception is raised (see - "Control.Exception"). + "Control.Exception"). However, it is only atomic if there are no + other producers for this 'MVar'. -} {-# INLINE withMVar #-} -- inlining has been reported to have dramatic effects; see @@ -96,9 +194,11 @@ return b {-| - A safe wrapper for modifying the contents of an 'MVar'. Like 'withMVar', - 'modifyMVar' will replace the original contents of the 'MVar' if an - exception is raised during the operation. + An exception-safe wrapper for modifying the contents of an 'MVar'. + Like 'withMVar', 'modifyMVar' will replace the original contents of + the 'MVar' if an exception is raised during the operation. This + function is only atomic if there are no other producers for this + 'MVar'. -} {-# INLINE modifyMVar_ #-} modifyMVar_ :: MVar a -> (a -> IO a) -> IO ()
participants (9)
-
Bertram Felgenhauer
-
Don Stewart
-
Edward Z. Yang
-
Jan-Willem Maessen
-
Jason Dagit
-
Malcolm Wallace
-
Mitar
-
Neil Brown
-
Stefan Monnier