<div dir="ltr">Hi all,<div><br></div><div style>This proposal is related to <a href="http://hackage.haskell.org/trac/ghc/ticket/1381">http://hackage.haskell.org/trac/ghc/ticket/1381</a>, which Simon Marlow closed through commit <a href="https://github.com/ghc/ghc/commit/02c4ab049adeb77b8ee0e3b98fbf0f3026eee453">https://github.com/ghc/ghc/commit/02c4ab049adeb77b8ee0e3b98fbf0f3026eee453</a>. </div>
<div style><br></div><div style>The problem, in a nutshell, is "how do we terminate a code snippet started with runStmt"? Before Simon's patch the only way was to disable ghc's sandboxing, so that the snippet would run in the same thread as the thread that called runStmt in the first place, and then send an asynchronous exception to that thread. This is the approach we used to take. It's a little tricky to get right (have to make sure that these exceptions are thrown only at the right times), but we thought we had it working okay.</div>
<div style><br></div><div style>Until, that is, we realized we had a very nasty problem: snippets were being unexpected interrupted. To debug this, we introduced a CustomUserInterruption data type to serve as the exception that we were throwing. This had two purposes: first, we would be use that if we saw a CustomUserInterrupt that it could only have come from one particular throw, and second, the CustomUserInterrupt carried an integer which was incremented on every throw so that we never threw the same exception twice.</div>
<div style><br></div><div style>What we realized is that snippets were being interrupted by *old* exceptions; that is, exceptions that we had thrown to *previous* snippets (and had been caught too). This should obviously never happen. Ian managed to reproduce this behaviour in a completely different setting (<a href="http://hackage.haskell.org/trac/ghc/ticket/5902#comment:5">http://hackage.haskell.org/trac/ghc/ticket/5902#comment:5</a>) and we think that something similar (unsafePerformIO related) must be happening inside ghc. </div>
<div style><br></div><div style>Moreover, a further conjecture is that this only happens when runStmt is still compiling the snippet to be executed (as opposed to the snippet actually executing) -- i.e., that the exception gets stuck in the bowels of ghc somewhere. We don't have any hard evidence for this, other than the problem has not appeared again with the proposed patch (but that might be luck, as it depends on timing).</div>
<div style><br></div><div style>The patch as we currently have it is against 7.4.2, so pre Simon's change for the sandbox behaviour -- but I don't think that Simon's changes affect the above problem. The core of our patch is</div>
<div style><br></div><div style><div><font face="courier new, monospace">-sandboxIO :: DynFlags -> MVar Status -> IO [HValue] -> IO Status</font></div><div><font face="courier new, monospace">-sandboxIO dflags statusMVar thing =</font></div>
<div><font face="courier new, monospace">+sandboxIO :: DynFlags -> MVar Status -> MVar (Maybe ThreadId) -> IO [HValue] -> IO Status</font></div><div><font face="courier new, monospace">+sandboxIO dflags statusMVar tidMVar thing =</font></div>
<div><font face="courier new, monospace"> mask $ \restore -> -- fork starts blocked</font></div><div><font face="courier new, monospace">- let runIt = liftM Complete $ try (restore $ rethrow dflags thing)</font></div>
<div><font face="courier new, monospace">+ let thing' = gbracket (myThreadId >>= putMVar tidMVar . Just)</font></div><div><font face="courier new, monospace">+ (\() -> modifyMVar_ tidMVar (\_ -> return Nothing))</font></div>
<div><font face="courier new, monospace">+ (\() -> thing)</font></div><div><font face="courier new, monospace">+ runIt = liftM Complete $ try (restore $ rethrow dflags thing')</font></div>
<div><font face="courier new, monospace"> in if dopt Opt_GhciSandbox dflags</font></div><div><font face="courier new, monospace"> then do tid <- forkIO $ do res <- runIt</font></div><div><font face="courier new, monospace"> putMVar statusMVar res -- empty: can't block</font></div>
<div><br></div><div style>That is, sandboxIO takes an additional MVar (Maybe ThreadId):</div><div style><ol style><li style>Initially this MVar should be empty. The MVar gets initialized to Just the Thread ID of the sandbox when the user code starts running. This means that if an secondary thread attempts to read the MVar (in order to kill the snippet), that secondary thread will block until the user code starts running -- it will not interrupt ghc when compiling the snippet.</li>
<li style>When the user code exists the MVar is updated to be Nothing. This means that if the auxiliary thread reads the MVar and finds Nothing it knows that the snippet has already terminated.</li><li style>When the auxiliary thread finds Just a thread ID (it must use withMVar rather than readMVar to avoid a race condition) it can safely throw the asynchronous exception.</li>
</ol><div style>The remainder of the patch just propagates these changes up so that runStmt takes this MVar as an argument, too. Probably when integrating the patch into ghc it would be better to leave runStmt along and provide a runStmt' that takes the additional argument.</div>
<div style><br></div><div style>Again, any and all feedback on the above would be appreciated.</div><div style><br></div><div style>Edsko</div></div></div></div>