[darcs-users] [patch455] Generalize withSignalsBlocked and withGutsOf

Eric Kow kowey at darcs.net
Thu Dec 23 12:29:40 UTC 2010


Sorry, I'm afraid I don't understand enough stuff to review this patch
effectively. :-(

I'll just lay all my little bits and pieces on the table and hope that
they can be swept into some coherent whole.

Gabriel gave me some helpful context yesterday:

  The context is that you want to be able to "restore to the original
  state" after you block some threads, that it you want to block the
  threads but when you come back, you don't want to unblock
  unconditionally because the threads may already have been blocked.

Furthermore, he told me that there will be no change to block/unblock;
they are just deprecated in GHC 7 and we'll have to use mask/unmask
instead.  That's a bit of a relief from a patch review standpoint.

General confusion 1: unblock function vs unblocking
----------------------------------------------------------------------
I'm not entirely sure I understand, though.  I think part of my
confusion stems from the existence of a function called 'unblock'
as opposed to the general effect of disabling exceptions.  So
when we say 'unblock' here, we just mean "disable"

As a sanity check, if we ignore the existence of the unblock function,
does that mean that the block behaviour of

  1. disables asynchronous exceptions
  2. job
  3. re-enables asynchronous exceptions

completely disregards whether or not asynchronous exceptions are already
blocked on the current thread?  Instead of keeping track of blockedness
counts or something, it just flips the switch on and off?

I guess so because the http://hackage.haskell.org/trac/ghc/ticket/1036
talks about "perhaps making block/unblock count nesting". It's kind of
funny because the "withFoo" style of block implies that I don't have to
worry about this...

General confusion 2: what is this function for?
----------------------------------------------------------------------
I have some silly/ignorant questions about terminology and what not.

Just taking the "before" version of this code.

> -withSignalsBlocked :: IO () -> IO ()
> -withSignalsBlocked job = (block job) `catchSignal` couldnt_do
> -    where couldnt_do s | s == sigINT = oops "interrupt"
> -                       | s ==  sigHUP = oops "HUP"
> -                       | s ==  sigABRT = oops "ABRT"
> -                       | s ==  sigALRM = oops "ALRM"
> -                       | s ==  sigTERM = oops "TERM"
> -                       | s ==  sigPIPE = return ()
> -                       | otherwise = oops "unknown signal"
> -          oops s = hPutStrLn stderr $ "Couldn't handle " ++ s ++
> -                   " since darcs was in a sensitive job."

If I'm reading my textbook right [1], this function is a bit of a
misnomer in two ways.

First, it's not literally blocking signals in the sense of "don't
deliver these signals yet"; it's actually accepting the signals and
dropping them, notifying the user that it has done so.  

Second, this isn't just about signals.  I was puzzling over the role of
the 'block' in the (block job).  It occurred to me that actually the
reason we disable asynchronous exceptions is not related to the dropping
signals on the floor task; it's just because Darcs is in a sensitive job
and you really don't want it interrupted at all.  So really the function
is just a general goAwayImBusy

[1] friend loaned me his Advanced Programming in the Unix Environment

Generalize withSignalsBlocked and withGutsOf
--------------------------------------------
> ] hunk ./src/Darcs/Repository/Internal.hs 660
> -withGutsOf :: Repository p C(r u t) -> IO () -> IO ()
> +withGutsOf :: Repository p C(r u t) -> IO a -> IO a

No surprises here

> -withSignalsBlocked :: IO () -> IO ()
> -withSignalsBlocked job = (block job) `catchSignal` couldnt_do

> +withSignalsBlocked :: IO a -> IO a
> +withSignalsBlocked job = block (job >>= \r ->
> +                           unblock(return r `catchSignal` couldnt_do r))

Things I think I do understand:

A. wanting to return r, and having to do it from the handler too

B. pushing the signal handler into
      >>= \r -> return r `catchSignal` couldnt_do r
   just to get r in scope of the handler

Three things I don't really understand:

1. Re B: why is it OK to go from
      catchSignal job couldnt_do
   to
      job >>= \r -> catchSignal ...
   Will signals still be caught even if job isn't wrapped by
   the catch? Why? Is it some sort of laziness-related thing?

2. Why do we wrap (return r `catchSignal` couldnt_do r) with an
   unblock?  It's not for the sake of couldnt_do as far as I can tell,
   since catch puts an implicit block around the exception handler.

   So my only guess is that for some reason you want there to be some
   gap where asynchronous exceptions are enabled but you are still
   catching/dropping signals.
   
   In other words, BEFORE:

     catch
      (1. disable exceptions
       2. job
       3. enable exceptions
       4. [GAP!])

   WRONG AFTER

     1. disable exceptions
     2. catch job (only signals raised from this thread are caught)
     3. enable exceptions

   BETTER AFTER

     1. disable exceptions
     2a. job
     2b. enable exceptions
     2c. [GAP!]
     2d. catch return
     2e. (disable exceptions)
     3. (enable exceptions)

3. What's the link between this code and what Gabriel said about
   only selectively re-enabling exceptions if they were already
   enabled before hand?  If my understanding in 'General confusion 1'
   is correct, then doesn't the block still brutally re-enable
   exceptions after we're done anyway?
   
   Does the finesse come from the unblock I was asking about in 2?  That
   only seems to guarantee that we have explicitly asked for exceptions
   to be disable during block before go and disable them

The rest of this code doesn't need much comment, just that want to
return the job result even if a signal was caught/dropped

> -    where couldnt_do s | s == sigINT = oops "interrupt"
> -                       | s ==  sigHUP = oops "HUP"
> -                       | s ==  sigABRT = oops "ABRT"
> -                       | s ==  sigALRM = oops "ALRM"
> -                       | s ==  sigTERM = oops "TERM"
> -                       | s ==  sigPIPE = return ()
> -                       | otherwise = oops "unknown signal"
> -          oops s = hPutStrLn stderr $ "Couldn't handle " ++ s ++
> -                   " since darcs was in a sensitive job."

> +    where couldnt_do r s | s == sigINT = oops "interrupt" r
> +                         | s ==  sigHUP = oops "HUP" r
> +                         | s ==  sigABRT = oops "ABRT" r
> +                         | s ==  sigALRM = oops "ALRM" r
> +                         | s ==  sigTERM = oops "TERM" r
> +                         | s ==  sigPIPE = return r
> +                         | otherwise = oops "unknown signal" r
> +          oops s r = do hPutStrLn stderr $ "Couldn't handle " ++ s ++
> +                          " since darcs was in a sensitive job."
> +                        return r

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, try +44 (0)1273 64 2905 or
xmpp:kowey at jabber.fr (Jabber or Google Talk only)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20101223/d45ea325/attachment.pgp>


More information about the darcs-users mailing list