[darcs-devel] [patch455] Generalize withSignalsBlocked and withGutsOf
Eric Kow
bugs at darcs.net
Thu Dec 23 12:39:58 UTC 2010
Eric Kow <kowey at darcs.net> added the comment:
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)
__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch455>
__________________________________
More information about the darcs-devel
mailing list