#2910: throwTo can block indefinitely when target thread finishes with
exceptions
blocked
---------------------------------+------------------------------------------
Reporter: int-e | Owner: igloo
Type: merge | Status: new
Priority: normal | Milestone: 6.10.2
Component: Runtime System | Version: 6.10.1
Severity: normal | Resolution:
Keywords: | Difficulty: Easy (1 hr)
Testcase: | Os: Unknown/Multiple
Architecture: Unknown/Multiple |
---------------------------------+------------------------------------------
Comment (by int-e):
Replying to [comment:4 simonmar]:
> Thanks - it's nice to have someone else looking at this code!
You're welcome.
> `lockTSO()` doesn't lock the `what_next` field, only the
`blocked_exceptions` field, so I think this change is not necessary.
Ah, I see. But I believe that the change still makes sense - see below.
> Threads that fall through the cracks and end up on the
blocked_exceptions list of a finished or blocked target thread are
supposed to be caught by the GC ...
And that's the part I missed, although admittedly I'm a bit unhappy about
waiting for the next GC. Does the RTS perform a GC when it finds no other
work? In any case, there'll be some wait.
Now I believe we can prevent this from happening, with those two hunks
above. The key idea is that once the {{{what_next}}} field is set to
{{{ThreadComplete}}} or {{{ThreadKilled}}}, it will not be modified again.
As you wrote in the comment in {{{scheduleHandleThreadFinished}}},
{{{what_next}}} has already been set when
{{{awakenBlockedExceptionQueue}}} is called. So the only scenario we have
to prevent is that a thread throwing an exception finds its target
running, and then adds itself to the target's {{{blocked_exception}}}
queue, with the target thread completing and running
{{{awakenExceptionQueue}}} inbetween those two steps.
This can be accomplished by making {{{awakenExceptionQueue}}} take the TSO
lock every time, *and* checking whether the target has finished between
taking the TSO lock and calling {{{blockedThrowTo}}} for all calls to
{{{blockedThrowTo}}}, unless we can prove that the thread cannot finish in
the meantime.
My changes only covered the {{{NotBlocked}}} case. I believe that in the
{{{Blocked*}}} cases, the thread cannot finish in the meantime (they lock
the TSO, directly or indirectly, and then check that the thread is still
blocked - which implies that it has not finished), but I'm 100% not
certain.
To summarize: We would not use the TSO lock to protect the {{{what_next}}}
field - we'd use (or abuse?) it to prevent a specific race between
{{{blockedThrowTo}}} and {{{awakenExceptionQueue}}}.
I think the benefits are clear: We avoid one case of the RTS having to
wait for a GC.
The cost seems bearable: {{{awakenExceptionQueue}}} is only called when a
thread finishes or when it returns from a C call (and in the latter case,
we could continue to use the old variant). Both cases aren't exactly fast
paths. Then there's a cost in code complexity (the reasoning is fairly
tricky) - but that's your judgement call.
--
Ticket URL: <http://hackage.haskell.org/trac/ghc/ticket/2910#comment:5>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler_______________________________________________
Glasgow-haskell-bugs mailing list
Glasgow-haskell-bugs@haskell.org
http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
