Skip to content

Commit

Permalink
Fix space leak in dynamic event switching (#256)
Browse files Browse the repository at this point in the history
Currently we have a nasty space leak in dynamic event switching. The crux of the problem is that we have a child event that needs to have a new parent. When we change the parent of a node, we need to create a weak pointer to the new parent and store it in the child's list of parents, and we need to store a weak pointer to the child in the new parent node.

Currently `connectChild` does this by creating a new weak pointer to the child whenever the parent changes:

```haskell
w <- mkWeakNodeValue child child
```

Here we create a weak pointer to the child `SomeNode`, kept alive by the underlying `IORef` that backs `child`.

The problem with this approach is that the `child` `IORef` may be reachable throughout the entire duration of the program execution (e.g., `child` is connected to a top-level `reactimate`). This is problematic, because a weak pointer is kept alive by the garbage collector if the key of the weak pointer is alive *regardless* of whether anyone actually has a reference to the weak pointer itself. In the case of `switchE`, this means any time we switch a child to have a new parent, we allocate one more weak pointer that can never be garbage collected.

The fix to allow the old weak pointers to be garbage collected is to call `finalize` on them, which we now do in `removeParents`. `removeParents` is responsible for removing a child from all current parents, so when we remove ourself from a parent (removing the weak pointer from the parent), we finalize the weak pointer.

Fixes #253, #152.
  • Loading branch information
ocharles authored Jun 16, 2022
1 parent 9f189f1 commit 7b1718b
Showing 1 changed file with 12 additions and 1 deletion.
13 changes: 12 additions & 1 deletion reactive-banana/src/Reactive/Banana/Prim/Low/Dependencies.hs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,18 @@ removeParents child = do
forM_ _parentsP $ \w -> do
Just (P parent) <- deRefWeak w -- get parent node
finalize w -- severe connection in garbage collector
let isGoodChild w = not . maybe True (== P child) <$> deRefWeak w
let isGoodChild w = deRefWeak w >>= \x ->
case x of
Just y | y /= P child -> return True
_ -> do
-- The old parent refers to this child. In this case we'll remove
-- this child from the parent, but we also need to finalize the
-- weak pointer that points to the child. We need to do this because
-- otherwise the weak pointer will stay alive (even though it's
-- unreachable) for as long as the child is alive
-- https://github.com/HeinrichApfelmus/reactive-banana/pull/256
finalize w
return False
new <- filterM isGoodChild . _childrenP =<< readRef parent
modify' parent $ set childrenP new
-- replace parents by empty list
Expand Down

0 comments on commit 7b1718b

Please sign in to comment.