#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 simonmar):
Replying to [comment:3 int-e]:
> As the patches I just attached suggest, this race is not completely
fixed. (I'm pretty certain - Conal's TestRace program locks up without the
first patch, but works fine so far with it. I also have a modified version
that logs thread creation and throwTo and shows the program lock up with
all threads finished except the main thread, which is blocked on an
exception.)
>
> The second patch contains changes unrelated to this bug which I'm not
100% certain about - but they felt necessary.
Thanks - it's nice to have someone else looking at this code!
In response to your patches:
{{{
hunk ./rts/RaiseAsync.c 418
// Unblocking BlockedOnSTM threads requires the TSO to be
// locked; see STM.c:unpark_tso().
if (target->why_blocked != BlockedOnSTM) {
+ unlockTSO(target);
goto retry;
}
if ((target->flags & TSO_BLOCKEX) &&
}}}
well spotted.
{{{
hunk ./rts/RaiseAsync.c 440
// thread is blocking exceptions, and block on its
// blocked_exception queue.
lockTSO(target);
+ if (target->why_blocked != BlockedOnCCall &&
+ target->why_blocked != BlockedOnCCall_NoUnblockExc) {
+ unlockTSO(target);
+ return;
+ }
blockedThrowTo(cap,source,target);
*out = target;
return THROWTO_BLOCKED;
}}}
again, well spotted - except that we want `goto retry` rather than
`return`.
{{{
hunk ./rts/RaiseAsync.c 267
target = target->_link;
goto retry;
}
+ // The thread may also have finished in the meantime.
+ if (target->what_next == ThreadKilled
+ || target->what_next == ThreadComplete) {
+ unlockTSO(target);
+ return THROWTO_SUCCESS;
+ }
blockedThrowTo(cap,source,target);
*out = target;
return THROWTO_BLOCKED;
}}}
`lockTSO()` doesn't lock the `what_next` field, only the
`blocked_exceptions` field, so I think this change is not necessary.
{{{
hunk ./rts/RaiseAsync.c 555
void
awakenBlockedExceptionQueue (Capability *cap, StgTSO *tso)
{
+ lockTSO(tso);
+ // Taking the tso lock before the following check assures that we
+ // wait for any throwTo that may just be adding a new thread to the
+ // queue. This is essential, because we may not get another chance
+ // to wake up that thread.
if (tso->blocked_exceptions != END_TSO_QUEUE) {
hunk ./rts/RaiseAsync.c 561
- lockTSO(tso);
awakenBlockedQueue(cap, tso->blocked_exceptions);
tso->blocked_exceptions = END_TSO_QUEUE;
hunk ./rts/RaiseAsync.c 563
- unlockTSO(tso);
}
hunk ./rts/RaiseAsync.c 564
+ unlockTSO(tso);
}
}}}
This is not necessary. However, while figuring out why, I did find the
real bug. 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 (see comments at line 216 in
`MarkWeak.c`). However, this wasn't working in the case when the target
thread had finished, because `maybePerformBlockedException()` wasn't
handling the `ThreadComplete` or `ThreadKilled` case, so I've fixed that.
--
Ticket URL: <http://hackage.haskell.org/trac/ghc/ticket/2910#comment:4>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler_______________________________________________
Glasgow-haskell-bugs mailing list
[email protected]
http://www.haskell.org/mailman/listinfo/glasgow-haskell-bugs
