From 32c4e7a4f96f26d48bfe42501e36fa08ed03c990 Mon Sep 17 00:00:00 2001
From: Ben Gamari <ben@smart-cactus.org>
Date: Mon, 6 Jul 2015 19:29:21 +0200
Subject: [PATCH] Add more discussion of black-holing logic for #10414
Signed-off-by: Ben Gamari <ben@smart-cactus.org>
---
compiler/codeGen/StgCmmClosure.hs | 68 +++++++++++++++++++++++++++----
1 file changed, 59 insertions(+), 9 deletions(-)
diff --git a/compiler/codeGen/StgCmmClosure.hs b/compiler/codeGen/StgCmmClosure.hs
index 984e704e3a43..850632c3b1bf 100644
--- a/compiler/codeGen/StgCmmClosure.hs
+++ b/compiler/codeGen/StgCmmClosure.hs
@@ -756,16 +756,10 @@ mkClosureInfo dflags is_static id lf_info tot_wds ptr_wds val_descr
-- is unconditionally disabled. -- krc 1/2007
--
--
--- A single-entry (non-updatable) thunk can actually be entered
--- more than once in a parallel program, if work is duplicated
--- by two threads both entering the same updatable thunk before
--- the other has blackholed it. So, we must not eagerly
--- blackhole non-updatable thunks, or the second thread to
--- enter one will become blocked indefinitely. (They are not
--- blackholed by lazy blackholing either, since they have no
--- associated update frame.) See Trac #10414.
-
-- Static closures are never themselves black-holed.
+--
+-- We also never black-hole non-updatable thunks.
+-- See Note [Black-holing non-updatable thunks]
blackHoleOnEntry :: ClosureInfo -> Bool
blackHoleOnEntry cl_info
@@ -779,6 +773,62 @@ blackHoleOnEntry cl_info
LFThunk _ _no_fvs updatable _ _ -> updatable
_other -> panic "blackHoleOnEntry" -- Should never happen
+{-
+Note [Black-holing non-updatable thunks]
+=========================================
+
+We cannot black-hole non-updatable thunks otherwise we run into issues like
+#10414. A single-entry (non-updatable) thunk can actually be entered more than
+once in a parallel program, if work is duplicated by two threads both entering
+the same updatable thunk before the other has blackholed it. So, we must not
+eagerly blackhole non-updatable thunks, or the second thread to enter one will
+become blocked indefinitely. (They are not blackholed by lazy blackholing
+either, since they have no associated update frame.)
+
+For instance, let's consider the following value (in pseudo-Core, example due to
+Reid Barton),
+
+ x = \u [] concat [[1], []]
+
+with the following definitions,
+
+ concat x = case x of
+ [] -> []
+ (:) x xs -> (++) x (concat xs)
+
+ (++) xs ys = case xs of
+ [] -> ys
+ (:) x rest -> (:) x ((++) rest ys)
+
+Where we use the syntax @\u []@ to denote an updatable thunk and @\s []@ to
+denote a single-entry (i.e. non-updatable) thunk. After a thread evaluates @x@
+to WHNF and calls @(++)@ the heap will contain the following thunks,
+
+ x = 1 : y
+ y = \u [] (++) [] z
+ z = \s [] concat []
+
+Now that the stage is set, consider the follow evaluations by two racing threads
+A and B,
+
+ 1. Both threads enter @y@ before either is able to replace it with an
+ indirection
+
+ 2. Thread A does the case analysis in @(++)@ and consequently enters @z@,
+ replacing it with a black-hole
+
+ 3. At some later point thread B does the same case analysis and also attempts
+ to enter @z@. However, it finds that it has been replaced with a black-hole
+ so it blocks.
+
+ 4. Thread A eventually finishes evaluating @z@ (to @[]@) and updates @y@
+ accordingly. It does *not* update @z@, however, as it is single-entry. This
+ leaves Thread B blocked forever on a black-hole which will never be
+ updated.
+
+To avoid this sort of condition we never black-hole non-updatable thunks.
+-}
+
isStaticClosure :: ClosureInfo -> Bool
isStaticClosure cl_info = isStaticRep (closureSMRep cl_info)
--
GitLab