[project @ 2001-07-06 14:11:38 by simonmar]
authorsimonmar <unknown>
Fri, 6 Jul 2001 14:11:38 +0000 (14:11 +0000)
committersimonmar <unknown>
Fri, 6 Jul 2001 14:11:38 +0000 (14:11 +0000)
Fix a couple of nasty bugs in the take/putMVar implementation.

Now we keep the invariant that a full MVar only has blocked putMVars
on its queue, and an empty MVar only has blocked takeMVars on its
queue.  It was the absence of this invariant that led to accidental
deadlock before.

The second bug is that there was a window between a blocked thread
being restarted and it actually retrying the takeMVar/putMVar
operation when it could receive an exception, which would also lead to
deadlock.

The solution to both these problems (as suggested by Simon P.J.) is to
atomically wake up and perform the next blocked putMVar when we do a
take, and vice versa.  As a side effect, takeMVar & putMVar should be
much faster when blocking & restarting, because we now shortcut the
retrying of the blocked operation and we use a more specialised stack
layout for the blocked thread.  Unfortunately, things got more
complicated too, but there are comments explaining what's going on.

ghc/rts/HeapStackCheck.h
ghc/rts/HeapStackCheck.hc
ghc/rts/PrimOps.hc

index 72a8ce0..1bbccd7 100644 (file)
@@ -1,5 +1,5 @@
 /* -----------------------------------------------------------------------------
- * $Id: HeapStackCheck.h,v 1.6 2000/12/14 15:19:47 sewardj Exp $
+ * $Id: HeapStackCheck.h,v 1.7 2001/07/06 14:11:38 simonmar Exp $
  *
  * (c) The GHC Team, 1998-1999
  *
@@ -7,6 +7,7 @@
  *
  * ---------------------------------------------------------------------------*/
 
+
 EXTFUN(stg_gc_entertop);
 EXTFUN(stg_gc_enter_1_hponly);
 EXTFUN(stg_gc_enter_1);
@@ -18,19 +19,28 @@ EXTFUN(stg_gc_enter_6);
 EXTFUN(stg_gc_enter_7);
 EXTFUN(stg_gc_enter_8);
 EXTFUN(stg_gc_seq_1);
-EXTFUN(stg_gc_noregs);
-EXTFUN(stg_gc_unpt_r1_entry);
-EXTFUN(stg_gc_unpt_r1);
-EXTFUN(stg_gc_unbx_r1_entry);
-EXTFUN(stg_gc_unbx_r1);
-EXTFUN(stg_gc_f1_entry);
-EXTFUN(stg_gc_f1);
-EXTFUN(stg_gc_d1_entry);
-EXTFUN(stg_gc_d1);
-EXTFUN(stg_gc_ut_1_0_entry);
-EXTFUN(stg_gc_ut_1_0);
-EXTFUN(stg_gc_ut_0_1_entry);
-EXTFUN(stg_gc_ut_0_1);
+
+EI_(stg_gc_noregs_ret_info);
+EF_(stg_gc_noregs);
+
+EI_(stg_gc_unpt_r1_ret_info);
+EF_(stg_gc_unpt_r1);
+
+EI_(stg_gc_unbx_r1_ret_info);
+EF_(stg_gc_unbx_r1);
+
+EI_(stg_gc_f1_ret_info);
+EF_(stg_gc_f1);
+
+EI_(stg_gc_d1_ret_info);
+EF_(stg_gc_d1);
+
+EI_(stg_gc_ut_1_0_ret_info);
+EF_(stg_gc_ut_1_0);
+
+EI_(stg_gc_ut_0_1_ret_info);
+EF_(stg_gc_ut_0_1);
+
 EXTFUN(stg_chk_0);
 EXTFUN(stg_chk_1);
 EXTFUN(stg_chk_1n);
@@ -50,3 +60,5 @@ EXTFUN(stg_yield_to_interpreter);
 EXTFUN(stg_gen_block);
 EXTFUN(stg_block_noregs);
 EXTFUN(stg_block_1);
+EXTFUN(stg_block_takemvar);
+EXTFUN(stg_block_putmvar);
index 8d0c46a..72ca553 100644 (file)
@@ -1,5 +1,5 @@
 /* -----------------------------------------------------------------------------
- * $Id: HeapStackCheck.hc,v 1.16 2001/03/23 16:36:21 simonmar Exp $
+ * $Id: HeapStackCheck.hc,v 1.17 2001/07/06 14:11:38 simonmar Exp $
  *
  * (c) The GHC Team, 1998-1999
  *
   R1.i = ThreadBlocked;                        \
   JMP_(StgReturn);
 
-
 /* -----------------------------------------------------------------------------
    Heap Checks
    -------------------------------------------------------------------------- */
@@ -663,11 +662,11 @@ EXTFUN(stg_gc_noregs)
 
 /*-- R1 is boxed/unpointed -------------------------------------------------- */
 
-INFO_TABLE_SRT_BITMAP(stg_gc_unpt_r1_info, stg_gc_unpt_r1_entry, 0/*BITMAP*/, 
+INFO_TABLE_SRT_BITMAP(stg_gc_unpt_r1_ret_info, stg_gc_unpt_r1_ret, 0/*BITMAP*/, 
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 
-EXTFUN(stg_gc_unpt_r1_entry)
+EXTFUN(stg_gc_unpt_r1_ret)
 {
   FB_
   R1.w = Sp[0];
@@ -681,19 +680,19 @@ EXTFUN(stg_gc_unpt_r1)
   FB_
   Sp -= 2;
   Sp[1] = R1.w;
-  Sp[0] = (W_)&stg_gc_unpt_r1_info;
+  Sp[0] = (W_)&stg_gc_unpt_r1_ret_info;
   GC_GENERIC
   FE_
 }
 
 /*-- R1 is unboxed -------------------------------------------------- */
 
-INFO_TABLE_SRT_BITMAP(stg_gc_unbx_r1_info, stg_gc_unbx_r1_entry, 1/*BITMAP*/,
+INFO_TABLE_SRT_BITMAP(stg_gc_unbx_r1_ret_info, stg_gc_unbx_r1_ret, 1/*BITMAP*/,
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 /* the 1 is a bitmap - i.e. 1 non-pointer word on the stack. */
 
-EXTFUN(stg_gc_unbx_r1_entry)
+EXTFUN(stg_gc_unbx_r1_ret)
 {
   FB_
   R1.w = Sp[0];
@@ -707,18 +706,18 @@ EXTFUN(stg_gc_unbx_r1)
   FB_
   Sp -= 2;
   Sp[1] = R1.w;
-  Sp[0] = (W_)&stg_gc_unbx_r1_info;
+  Sp[0] = (W_)&stg_gc_unbx_r1_ret;
   GC_GENERIC
   FE_
 }
 
 /*-- F1 contains a float ------------------------------------------------- */
 
-INFO_TABLE_SRT_BITMAP(stg_gc_f1_info, stg_gc_f1_entry, 1/*BITMAP*/,
+INFO_TABLE_SRT_BITMAP(stg_gc_f1_ret_info, stg_gc_f1_ret, 1/*BITMAP*/,
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 
-EXTFUN(stg_gc_f1_entry)
+EXTFUN(stg_gc_f1_ret)
 {
   FB_
   F1 = PK_FLT(Sp);
@@ -732,7 +731,7 @@ EXTFUN(stg_gc_f1)
   FB_
   Sp -= 2;
   ASSIGN_FLT(Sp+1, F1);
-  Sp[0] = (W_)&stg_gc_f1_info;
+  Sp[0] = (W_)&stg_gc_f1_ret_info;
   GC_GENERIC
   FE_
 }
@@ -747,11 +746,11 @@ EXTFUN(stg_gc_f1)
 #  define DBL_BITMAP 3
 #endif 
 
-INFO_TABLE_SRT_BITMAP(stg_gc_d1_info, stg_gc_d1_entry, DBL_BITMAP,
+INFO_TABLE_SRT_BITMAP(stg_gc_d1_ret_info, stg_gc_d1_ret, DBL_BITMAP,
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 
-EXTFUN(stg_gc_d1_entry)
+EXTFUN(stg_gc_d1_ret)
 {
   FB_
   D1 = PK_DBL(Sp);
@@ -765,7 +764,7 @@ EXTFUN(stg_gc_d1)
   FB_
   Sp -= 1 + sizeofW(StgDouble);
   ASSIGN_DBL(Sp+1,D1);
-  Sp[0] = (W_)&stg_gc_d1_info;
+  Sp[0] = (W_)&stg_gc_d1_ret_info;
   GC_GENERIC
   FE_
 }
@@ -800,11 +799,11 @@ EXTFUN(stg_gc_d1)
 
 /*---- R1 contains a pointer: ------ */
 
-INFO_TABLE_SRT_BITMAP(stg_gc_ut_1_0_info, stg_gc_ut_1_0_entry, 1/*BITMAP*/, 
+INFO_TABLE_SRT_BITMAP(stg_gc_ut_1_0_ret_info, stg_gc_ut_1_0_ret, 1/*BITMAP*/, 
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 
-EXTFUN(stg_gc_ut_1_0_entry)
+EXTFUN(stg_gc_ut_1_0_ret)
 {
   FB_
   R1.w = Sp[1];
@@ -819,18 +818,18 @@ EXTFUN(stg_gc_ut_1_0)
   Sp -= 3;
   Sp[2] = R1.w;
   Sp[1] = R2.w;
-  Sp[0] = (W_)&stg_gc_ut_1_0_info;
+  Sp[0] = (W_)&stg_gc_ut_1_0_ret_info;
   GC_GENERIC
   FE_
 }
 
 /*---- R1 contains a non-pointer: ------ */
 
-INFO_TABLE_SRT_BITMAP(stg_gc_ut_0_1_info, stg_gc_ut_0_1_entry, 3/*BITMAP*/, 
+INFO_TABLE_SRT_BITMAP(stg_gc_ut_0_1_ret_info, stg_gc_ut_0_1_ret, 3/*BITMAP*/, 
                      0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
                      RET_SMALL,, EF_, 0, 0);
 
-EXTFUN(stg_gc_ut_0_1_entry)
+EXTFUN(stg_gc_ut_0_1_ret)
 {
   FB_
   R1.w = Sp[1];
@@ -843,7 +842,7 @@ EXTFUN(stg_gc_ut_0_1)
 {
   FB_
   Sp -= 3;
-  Sp[0] = (W_)&stg_gc_ut_0_1_info;
+  Sp[0] = (W_)&stg_gc_ut_0_1_ret_info;
   Sp[1] = R2.w;
   Sp[2] = R1.w;
   GC_GENERIC
@@ -1215,3 +1214,71 @@ FN_(stg_block_1)
   BLOCK_ENTER;
   FE_
 }
+
+/* -----------------------------------------------------------------------------
+ * takeMVar/putMVar-specific blocks
+ *
+ * Stack layout for a thread blocked in takeMVar:
+ *      
+ *       ret. addr
+ *       ptr to MVar   (R1)
+ *       stg_block_takemvar_ret
+ *
+ * Stack layout for a thread blocked in putMVar:
+ *      
+ *       ret. addr
+ *       ptr to Value  (R2)
+ *       ptr to MVar   (R1)
+ *       stg_block_putmvar_ret
+ *
+ * See PrimOps.hc for a description of the workings of take/putMVar.
+ * 
+ * -------------------------------------------------------------------------- */
+
+INFO_TABLE_SRT_BITMAP(stg_block_takemvar_ret_info,  stg_block_takemvar_ret,
+                     0/*BITMAP*/, 0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
+                     RET_SMALL,, IF_, 0, 0);
+
+IF_(stg_block_takemvar_ret)
+{
+  FB_
+  R1.w = Sp[0];
+  Sp++;
+  JMP_(takeMVarzh_fast);
+  FE_
+}
+
+FN_(stg_block_takemvar)
+{
+  FB_
+  Sp -= 2;
+  Sp[1] = R1.w;
+  Sp[0] = (W_)&stg_block_takemvar_ret;
+  BLOCK_GENERIC;
+  FE_
+}
+
+INFO_TABLE_SRT_BITMAP(stg_block_putmvar_ret_info,  stg_block_putmvar_ret,
+                     0/*BITMAP*/, 0/*SRT*/, 0/*SRT_OFF*/, 0/*SRT_LEN*/, 
+                     RET_SMALL,, IF_, 0, 0);
+
+IF_(stg_block_putmvar_ret)
+{
+  FB_
+  R2.w = Sp[1];
+  R1.w = Sp[0];
+  Sp += 2;
+  JMP_(putMVarzh_fast);
+  FE_
+}
+
+FN_(stg_block_putmvar)
+{
+  FB_
+  Sp -= 3;
+  Sp[2] = R2.w;
+  Sp[1] = R1.w;
+  Sp[0] = (W_)&stg_block_putmvar_ret;
+  BLOCK_GENERIC;
+  FE_
+}
index a23561f..346705d 100644 (file)
@@ -1,5 +1,5 @@
 /* -----------------------------------------------------------------------------
- * $Id: PrimOps.hc,v 1.78 2001/03/26 13:43:05 simonmar Exp $
+ * $Id: PrimOps.hc,v 1.79 2001/07/06 14:11:38 simonmar Exp $
  *
  * (c) The GHC Team, 1998-2000
  *
@@ -808,6 +808,38 @@ FN_(yieldzh_fast)
   FE_
 }
 
+/* -----------------------------------------------------------------------------
+ * MVar primitives
+ *
+ * take & putMVar work as follows.  Firstly, an important invariant:
+ *
+ *    If the MVar is full, then the blocking queue contains only
+ *    threads blocked on putMVar, and if the MVar is empty then the
+ *    blocking queue contains only threads blocked on takeMVar.
+ *
+ * takeMvar:
+ *    MVar empty : then add ourselves to the blocking queue
+ *    MVar full  : remove the value from the MVar, and
+ *                 blocking queue empty     : return
+ *                 blocking queue non-empty : perform the first blocked putMVar
+ *                                            from the queue, and wake up the
+ *                                            thread (MVar is now full again)
+ *
+ * putMVar is just the dual of the above algorithm.
+ *
+ * How do we "perform a putMVar"?  Well, we have to fiddle around with
+ * the stack of the thread waiting to do the putMVar.  See
+ * stg_block_putmvar and stg_block_takemvar in HeapStackCheck.c for
+ * the stack layout, and the PerformPut and PerformTake macros below.
+ *
+ * It is important that a blocked take or put is woken up with the
+ * take/put already performed, because otherwise there would be a
+ * small window of vulnerability where the thread could receive an
+ * exception and never perform its take or put, and we'd end up with a
+ * deadlock.
+ *
+ * -------------------------------------------------------------------------- */
+
 FN_(newMVarzh_fast)
 {
   StgMVar *mvar;
@@ -830,6 +862,18 @@ FN_(newMVarzh_fast)
   FE_
 }
 
+#define PerformTake(tso, value) ({                     \
+    (tso)->sp[1] = (W_)value;                          \
+    (tso)->sp[0] = (W_)&stg_gc_unpt_r1_ret_info;       \
+  })
+
+#define PerformPut(tso) ({                             \
+    StgClosure *val = (StgClosure *)(tso)->sp[2];      \
+    (tso)->sp[2] = (W_)&stg_gc_noregs_ret_info;                \
+    (tso)->sp += 2;                                    \
+    val;                                               \
+  })
+
 FN_(takeMVarzh_fast)
 {
   StgMVar *mvar;
@@ -865,16 +909,21 @@ FN_(takeMVarzh_fast)
     /* unlock the MVar */
     mvar->header.info = &stg_EMPTY_MVAR_info;
 #endif
-    BLOCK(R1_PTR, takeMVarzh_fast);
+    JMP_(stg_block_takemvar);
   }
 
+  /* we got the value... */
   val = mvar->value;
-  mvar->value = (StgClosure *)&stg_END_TSO_QUEUE_closure;
 
-  /* wake up the first thread on the queue
-   */
   if (mvar->head != (StgTSO *)&stg_END_TSO_QUEUE_closure) {
+      /* There are putMVar(s) waiting... 
+       * wake up the first thread on the queue
+       */
       ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+
+      /* actually perform the putMVar for the thread that we just woke up */
+      mvar->value = PerformPut(mvar->head);
+
 #if defined(GRAN) || defined(PAR)
       /* ToDo: check 2nd arg (mvar) is right */
       mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
@@ -884,15 +933,23 @@ FN_(takeMVarzh_fast)
       if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
          mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
       }
+#ifdef SMP
+      /* unlock in the SMP case */
+      SET_INFO(mvar,&stg_FULL_MVAR_info);
+#endif
+      TICK_RET_UNBOXED_TUP(1);
+      RET_P(val);
+  } else {
+      /* No further putMVars, MVar is now empty */
+
+      /* do this last... we might have locked the MVar in the SMP case,
+       * and writing the info pointer will unlock it.
+       */
+      SET_INFO(mvar,&stg_EMPTY_MVAR_info);
+      mvar->value = (StgClosure *)&stg_END_TSO_QUEUE_closure;
+      TICK_RET_UNBOXED_TUP(1);
+      RET_P(val);
   }
-
-  /* do this last... we might have locked the MVar in the SMP case,
-   * and writing the info pointer will unlock it.
-   */
-  SET_INFO(mvar,&stg_EMPTY_MVAR_info);
-
-  TICK_RET_UNBOXED_TUP(1);
-  RET_P(val);
   FE_
 }
 
@@ -916,21 +973,28 @@ FN_(tryTakeMVarzh_fast)
   if (info == &stg_EMPTY_MVAR_info) {
 
 #ifdef SMP
-    /* unlock the MVar */
-    mvar->header.info = &stg_EMPTY_MVAR_info;
+      /* unlock the MVar */
+      SET_INFO(mvar,&stg_EMPTY_MVAR_info);
 #endif
 
-    /* HACK: we need a pointer to pass back, so we abuse NO_FINALIZER_closure */
-    RET_NP(0, &stg_NO_FINALIZER_closure);
+      /* HACK: we need a pointer to pass back, 
+       * so we abuse NO_FINALIZER_closure
+       */
+      RET_NP(0, &stg_NO_FINALIZER_closure);
   }
 
+  /* we got the value... */
   val = mvar->value;
-  mvar->value = (StgClosure *)&stg_END_TSO_QUEUE_closure;
 
-  /* wake up the first thread on the queue
-   */
   if (mvar->head != (StgTSO *)&stg_END_TSO_QUEUE_closure) {
+      /* There are putMVar(s) waiting... 
+       * wake up the first thread on the queue
+       */
       ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+
+      /* actually perform the putMVar for the thread that we just woke up */
+      mvar->value = PerformPut(mvar->head);
+
 #if defined(GRAN) || defined(PAR)
       /* ToDo: check 2nd arg (mvar) is right */
       mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
@@ -940,15 +1004,23 @@ FN_(tryTakeMVarzh_fast)
       if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
          mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
       }
+#ifdef SMP
+      /* unlock in the SMP case */
+      SET_INFO(mvar,&stg_FULL_MVAR_info);
+#endif
+      TICK_RET_UNBOXED_TUP(1);
+      RET_P(val);
+  } else {
+      /* No further putMVars, MVar is now empty */
+
+      /* do this last... we might have locked the MVar in the SMP case,
+       * and writing the info pointer will unlock it.
+       */
+      SET_INFO(mvar,&stg_EMPTY_MVAR_info);
+      mvar->value = (StgClosure *)&stg_END_TSO_QUEUE_closure;
+      TICK_RET_UNBOXED_TUP(1);
+      RET_P(val);
   }
-
-  /* do this last... we might have locked the MVar in the SMP case,
-   * and writing the info pointer will unlock it.
-   */
-  SET_INFO(mvar,&stg_EMPTY_MVAR_info);
-
-  TICK_RET_UNBOXED_TUP(1);
-  RET_NP(1,val);
   FE_
 }
 
@@ -981,34 +1053,42 @@ FN_(putMVarzh_fast)
 
 #ifdef SMP
     /* unlock the MVar */
-    mvar->header.info = &stg_FULL_MVAR_info;
+    SET_INFO(mvar,&stg_FULL_MVAR_info);
 #endif
-    BLOCK( R1_PTR | R2_PTR, putMVarzh_fast );
+    JMP_(stg_block_putmvar);
   }
   
-  mvar->value = R2.cl;
-
-  /* wake up the first thread on the queue, it will continue with the
-   * takeMVar operation and mark the MVar empty again.
-   */
   if (mvar->head != (StgTSO *)&stg_END_TSO_QUEUE_closure) {
-    ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+      /* There are takeMVar(s) waiting: wake up the first one
+       */
+      ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+
+      /* actually perform the takeMVar */
+      PerformTake(mvar->head, R2.cl);
+      
 #if defined(GRAN) || defined(PAR)
-    /* ToDo: check 2nd arg (mvar) is right */
-    mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
+      /* ToDo: check 2nd arg (mvar) is right */
+      mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
 #else
-    mvar->head = RET_STGCALL1(StgTSO *,unblockOne,mvar->head);
+      mvar->head = RET_STGCALL1(StgTSO *,unblockOne,mvar->head);
 #endif
-    if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
-      mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
-    }
+      if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
+         mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
+      }
+#ifdef SMP
+      /* unlocks the MVar in the SMP case */
+      SET_INFO(mvar,&stg_EMPTY_MVAR_info);
+#endif
+      JMP_(ENTRY_CODE(Sp[0]));
+  } else {
+      /* No further takes, the MVar is now full. */
+      mvar->value = R2.cl;
+      /* unlocks the MVar in the SMP case */
+      SET_INFO(mvar,&stg_FULL_MVAR_info);
+      JMP_(ENTRY_CODE(Sp[0]));
   }
 
-  /* unlocks the MVar in the SMP case */
-  SET_INFO(mvar,&stg_FULL_MVAR_info);
-
-  /* ToDo: yield here for better communication performance? */
-  JMP_(ENTRY_CODE(Sp[0]));
+  /* ToDo: yield afterward for better communication performance? */
   FE_
 }
 
@@ -1038,29 +1118,37 @@ FN_(tryPutMVarzh_fast)
     RET_N(0);
   }
   
-  mvar->value = R2.cl;
-
-  /* wake up the first thread on the queue, it will continue with the
-   * takeMVar operation and mark the MVar empty again.
-   */
   if (mvar->head != (StgTSO *)&stg_END_TSO_QUEUE_closure) {
-    ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+      /* There are takeMVar(s) waiting: wake up the first one
+       */
+      ASSERT(mvar->head->why_blocked == BlockedOnMVar);
+
+      /* actually perform the takeMVar */
+      PerformTake(mvar->head, R2.cl);
+      
 #if defined(GRAN) || defined(PAR)
-    /* ToDo: check 2nd arg (mvar) is right */
-    mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
+      /* ToDo: check 2nd arg (mvar) is right */
+      mvar->head = RET_STGCALL2(StgTSO *,unblockOne,mvar->head,mvar);
 #else
-    mvar->head = RET_STGCALL1(StgTSO *,unblockOne,mvar->head);
+      mvar->head = RET_STGCALL1(StgTSO *,unblockOne,mvar->head);
 #endif
-    if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
-      mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
-    }
+      if (mvar->head == (StgTSO *)&stg_END_TSO_QUEUE_closure) {
+         mvar->tail = (StgTSO *)&stg_END_TSO_QUEUE_closure;
+      }
+#ifdef SMP
+      /* unlocks the MVar in the SMP case */
+      SET_INFO(mvar,&stg_EMPTY_MVAR_info);
+#endif
+      JMP_(ENTRY_CODE(Sp[0]));
+  } else {
+      /* No further takes, the MVar is now full. */
+      mvar->value = R2.cl;
+      /* unlocks the MVar in the SMP case */
+      SET_INFO(mvar,&stg_FULL_MVAR_info);
+      JMP_(ENTRY_CODE(Sp[0]));
   }
 
-  /* unlocks the MVar in the SMP case */
-  SET_INFO(mvar,&stg_FULL_MVAR_info);
-
-  /* ToDo: yield here for better communication performance? */
-  RET_N(1);
+  /* ToDo: yield afterward for better communication performance? */
   FE_
 }