rts/RetainerProfile: Adding missing closure types to isRetainer
authorBen Gamari <bgamari.foss@gmail.com>
Tue, 19 Sep 2017 13:51:01 +0000 (09:51 -0400)
committerBen Gamari <ben@smart-cactus.org>
Tue, 19 Sep 2017 17:37:46 +0000 (13:37 -0400)
orzo in `#ghc` reported seeing a crash due to the retainer profiler encountering
a BLOCKING_QUEUE closure, which isRetainer didn't know about. I performed an
audit to make sure that all of the valid closure types were listed; they
weren't. This is my guess of how they should appear.

Test Plan: Validate

Reviewers: simonmar, austin, erikd

Reviewed By: simonmar

Subscribers: rwbarton, thomie

GHC Trac Issues: #14235

Differential Revision: https://phabricator.haskell.org/D3967

includes/rts/storage/ClosureTypes.h
rts/RetainerProfile.c

index 68cc919..240982c 100644 (file)
@@ -1,5 +1,5 @@
 /* ----------------------------------------------------------------------------
- * 
+ *
  * (c) The GHC Team, 1998-2005
  *
  * Closure Type Constants: out here because the native code generator
@@ -9,11 +9,12 @@
 
 #pragma once
 
-/* 
+/*
  * WARNING WARNING WARNING
  *
- * If you add or delete any closure types, don't forget to update
- * the closure flags table in rts/ClosureFlags.c.
+ * If you add or delete any closure types, don't forget to update the following,
+ *   - the closure flags table in rts/ClosureFlags.c
+ *   - isRetainer in rts/RetainerProfile.c
  */
 
 /* Object tag 0 raises an internal error */
index 1d5e923..7a9b9cc 100644 (file)
 #include "Stable.h" /* markStableTables */
 #include "sm/Storage.h" // for END_OF_STATIC_LIST
 
+/* Note [What is a retainer?]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~
+The definition of what sorts of things are counted as retainers is a bit hard to
+pin down. Intuitively, we want to identify closures which will help the user
+identify memory leaks due to thunks. In practice we also end up lumping mutable
+objects in this group for reasons that have been lost to time.
+
+The definition of retainer is implemented in isRetainer(), defined later in this
+file.
+*/
+
+
 /*
   Note: what to change in order to plug-in a new retainer profiling scheme?
     (1) type retainer in ../includes/StgRetainerProf.h
@@ -1022,6 +1034,9 @@ isRetainer( StgClosure *c )
     case MUT_VAR_DIRTY:
     case MUT_ARR_PTRS_CLEAN:
     case MUT_ARR_PTRS_DIRTY:
+    case SMALL_MUT_ARR_PTRS_CLEAN:
+    case SMALL_MUT_ARR_PTRS_DIRTY:
+    case BLOCKING_QUEUE:
 
         // thunks are retainers.
     case THUNK:
@@ -1069,17 +1084,21 @@ isRetainer( StgClosure *c )
     // closures. See trac #3956 for a program that hit this error.
     case IND_STATIC:
     case BLACKHOLE:
+    case WHITEHOLE:
         // static objects
     case FUN_STATIC:
         // misc
     case PRIM:
     case BCO:
     case ARR_WORDS:
+    case COMPACT_NFDATA:
         // STM
     case TREC_CHUNK:
         // immutable arrays
     case MUT_ARR_PTRS_FROZEN:
     case MUT_ARR_PTRS_FROZEN0:
+    case SMALL_MUT_ARR_PTRS_FROZEN:
+    case SMALL_MUT_ARR_PTRS_FROZEN0:
         return false;
 
         //
@@ -1089,11 +1108,15 @@ isRetainer( StgClosure *c )
         // legal objects during retainer profiling.
     case UPDATE_FRAME:
     case CATCH_FRAME:
+    case CATCH_RETRY_FRAME:
+    case CATCH_STM_FRAME:
     case UNDERFLOW_FRAME:
+    case ATOMICALLY_FRAME:
     case STOP_FRAME:
     case RET_BCO:
     case RET_SMALL:
     case RET_BIG:
+    case RET_FUN:
         // other cases
     case IND:
     case INVALID_OBJECT: