Re: [GHC] #15508: concprog001 fails with various errors

24 Sep 2018

      #15508: concprog001 fails with various errors
-------------------------------------+-------------------------------------
        Reporter:  osa1              |                Owner:  osa1
            Type:  bug               |               Status:  new
        Priority:  high              |            Milestone:  8.6.1
       Component:  Compiler          |              Version:  8.5
      Resolution:                    |             Keywords:
Operating System:  Unknown/Multiple  |         Architecture:
                                     |  Unknown/Multiple
 Type of failure:  Runtime crash     |            Test Case:
      Blocked By:                    |             Blocking:
 Related Tickets:  #15571            |  Differential Rev(s):  Phab:D5051
       Wiki Page:                    |  (reverted), Phab:D5165
-------------------------------------+-------------------------------------

Comment (by osa1):

 This is easy to fix, but I'm trying to understand the code:

 {{{
 // Revoke the message by replacing it with IND. We're not
 // locking anything here, so we might still get a TRY_WAKEUP
 // message from the owner of the blackhole some time in the
 // future, but that doesn't matter.
 ASSERT(target->block_info.bh->header.info == &stg_MSG_BLACKHOLE_info);
 OVERWRITE_INFO(target->block_info.bh, &stg_IND_info);
 raiseAsync(cap, target, msg->exception, false, NULL);
 return THROWTO_SUCCESS;
 }}}

 (in throwToMsg())

 Why we can make a MessageBlackHole an IND without setting the indirectee?
 Is this used to remove (by skipping) a MessageBlackHole from the queue?
 (because actually removing would be impossible as we don't have a `prev`
 field in `MessageBlackHole`?)

 I'm quite confused about how these two types are used:

 {{{
 typedef struct StgBlockingQueue_ {
     StgHeader   header;
     struct StgBlockingQueue_ *link; // here so it looks like an IND
     StgClosure *bh;  // the BLACKHOLE
     StgTSO     *owner;
     struct MessageBlackHole_ *queue;
 } StgBlockingQueue;

 typedef struct MessageBlackHole_ {
     StgHeader   header;
     struct MessageBlackHole_ *link;
     StgTSO     *tso;
     StgClosure *bh;
 } MessageBlackHole;
 }}}

 As far as I understand, a BLACKHOLE can become a BLOCKING_QUEUE (in
 `messageBlackHole()`), and `queue` of a BLOCKING_QUEUE is for threads that
 are blocked on this BLACKHOLE. But then

 - Why do we have a `bh` field in `messageBlackHole`? We could pass the
 `bh` as a parameter to `messageBlackHole()` and `bh` would the blackhole
 that we just looked at to find the `MessageBlackHole`.
 - Why do we need a list of `BLOCKING_QUEUE`s? Is this only to be able to
 implement `checkBlockingQueues()`? The comments around
 `checkBlockingQueues()` say

   {{{
   // If we update a closure that we know we BLACKHOLE'd, and the closure
   // no longer points to the current TSO as its owner, then there may be
   // an orphaned BLOCKING_QUEUE closure with blocked threads attached to
   // it.  We therefore traverse the BLOCKING_QUEUEs attached to the
   // current TSO to see if any can now be woken up.
   }}}

   but I don't understand how can owner of a BLACKHOLE not point to our
 capability if we BLACKHOLEd it.

 Could you say a few workds about this?

 I'll submit a patch but I don't know what to say in the commit message as
 I don't understand this code ...

-- 
Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15508#comment:18
GHC http://www.haskell.org/ghc/
The Glasgow Haskell Compiler