Memory leak with `reifyTypeable`

[mniip]

If a value is reified with reifyTypeable and then never reflected, its underlying StablePtr is leaked and never freed.

Example reproducer:

{- cabal:
build-depends: base, reflection
-}

import Control.Exception
import Data.Reflection

go :: Int -> IO ()
go 0 = pure ()
go n = reifyTypeable n $ \p -> do
  --evaluate (reflect p)
  go (n - 1)

main = go 10000000

Running this on my machine exhibits 160,031,904 bytes maximum residency (10 sample(s)). Uncommenting the evaluate reduces it to 35,984 bytes maximum residency (1 sample(s)).

[mniip]

It looks a similar thing was already caught by @treeowl in his patch in #45 here: https://github.com/ekmett/reflection/pull/45/files#diff-0bbfdb09906ad00c8717878ad908860f2fa0e5689ac8b89eef79057f5c57b1d2R455-R456

[RyanGlScott]

Well spotted. I have no idea how the current implementation of reifyTypeable works (seriously, just look at this monstrosity), so a closer look at #45 is warranted.

[mniip]

As far as I understand, it allocates a StablePtr pointing to the value, then converts the pointer to 8 bytes, whose values are lifted to the type level by "low tech means": the B class is a "known byte" class instantiated by 256 type constructors T0 through T255, and reflectByte calls its continuation with one of these types.

On the other side we reflect the byte values using B and assemble a StablePtr which we then deref.

A big nuance here is that for a given StablePtr, only one Reifies (Stable ...) a dictionary is ever created (inside reifyTypeable), and this dictionary is merely passed around. With this assumption in mind, reflect (which is a thunk in the dictionary) actually frees the StablePtr when evaluated, and the read value replaces the thunk.

A workaround is (as @treeowl does) is to immediately evaluate the reflect thunk when creating the dictionary. This will technically evaluate the argument to WHNF.

[RyanGlScott]

OK. If I understand correctly, would the following patch suffice to fix the space leak in the current version of reflection?

diff --git a/fast/Data/Reflection.hs b/fast/Data/Reflection.hs
index f5ded8a..6c41385 100644
--- a/fast/Data/Reflection.hs
+++ b/fast/Data/Reflection.hs
@@ -108,6 +108,7 @@ module Data.Reflection
     ) where
 
 import Control.Applicative
+import Control.Exception (evaluate)
 
 #ifdef MIN_VERSION_template_haskell
 import Control.Monad
@@ -597,16 +598,20 @@ reifyTypeable a k = unsafePerformIO $ do
 #endif
   p <- newStablePtr a
   let n = stablePtrToIntPtr p
-  reifyByte (fromIntegral n) (\s0 ->
-    reifyByte (fromIntegral (n `shiftR` 8)) (\s1 ->
-      reifyByte (fromIntegral (n `shiftR` 16)) (\s2 ->
-        reifyByte (fromIntegral (n `shiftR` 24)) (\s3 ->
-          reifyByte (fromIntegral (n `shiftR` 32)) (\s4 ->
-            reifyByte (fromIntegral (n `shiftR` 40)) (\s5 ->
-              reifyByte (fromIntegral (n `shiftR` 48)) (\s6 ->
-                reifyByte (fromIntegral (n `shiftR` 56)) (\s7 ->
-                  reflectBefore (fmap return k) $
-                    stable s0 s1 s2 s3 s4 s5 s6 s7))))))))
+  reifyByte (fromIntegral n) (\(s0 :: Proxy b0) ->
+    reifyByte (fromIntegral (n `shiftR` 8)) (\(s1 :: Proxy b1) ->
+      reifyByte (fromIntegral (n `shiftR` 16)) (\(s2 :: Proxy b2) ->
+        reifyByte (fromIntegral (n `shiftR` 24)) (\(s3 :: Proxy b3) ->
+          reifyByte (fromIntegral (n `shiftR` 32)) (\(s4 :: Proxy b4) ->
+            reifyByte (fromIntegral (n `shiftR` 40)) (\(s5 :: Proxy b5) ->
+              reifyByte (fromIntegral (n `shiftR` 48)) (\(s6 :: Proxy b6) ->
+                reifyByte (fromIntegral (n `shiftR` 56)) (\(s7 :: Proxy b7) ->
+                  reflectBefore
+                    (\s -> do
+                      -- Make sure we don't leak memory if `reflect` isn't otherwise used.
+                      _ <- evaluate (reflect :: Proxy (Stable (W b0 b1 b2 b3) (W b4 b5 b6 b7) a) -> a)
+                      return (k s))
+                    (stable s0 s1 s2 s3 s4 s5 s6 s7)))))))))
 
 
 data ReifiedMonoid a = ReifiedMonoid { reifiedMappend :: a -> a -> a, reifiedMempty :: a }

[mniip]

I think some of the syntax you used isn't supported all the way back to GHC 7.0, but otherwise yes.

Also there's again the issue of evaluating to WHNF. It can be worked around however by a little scheme where reifyTypeable actually does newStablePtr (Box a), and the implementation of Reifies (StablePtr _ _) evaluates to a Box a. Then we add another token type whose reflect simply unpacks the box, and that type is what's passed to the continuation in reifyTypeable.