Skip to content

Commit

Permalink
Implement experimental fix for GHCJS weak pointers
Browse files Browse the repository at this point in the history
  • Loading branch information
@HeinrichApfelmus
HeinrichApfelmus committed Oct 7, 2015
1 parent 3ac2dfc commit 211e221
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion reactive-banana/src/Reactive/Banana/Prim/Util.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ mkWeakIORefValueFinalizer r@(GHC.IORef (GHC.STRef r#)) v f = GHC.IO $ \s ->
case GHC.mkWeak# r# v f s of (# s1, w #) -> (# s1, GHC.Weak w #)

mkWeakIORefValue :: IORef a -> value -> IO (Weak value)
mkWeakIORefValue a b = mkWeakIORefValueFinalizer a b (return ())
mkWeakIORefValue a b = mkWeakIORefValueFinalizer a b $ touch a >> return ()
where
-- FIXME: The key is referenced in the finalizer for compatibility with GHCJS
touch ref = void $ readIORef ref

mkWeakRefValue :: MonadIO m => Ref a -> value -> m (Weak value)
mkWeakRefValue (Ref ref _) v = liftIO $ mkWeakIORefValue ref v
Expand Down

3 comments on commit 211e221

@alios
Copy link

@alios alios commented on 211e221 Oct 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi!
Just tried it, but it produces the same result for me, as with the head branch => It stops working after ~2 secs (aka when the GC) did run.

P.S. I created a patch here for the ghcjs runtime (shims) which fixes it by trying to restore the ghc seamntics, but it might be stil buggy and/or memory leaking (haven't worked on the ghcjs rts so far), but at least it seems to solve the problem for me with that FRP event propagation stops after GC run. But I also like the idea of exploring if there could be a fix on the reactive-banana side which works with the current semantics the ghcjs runtime implements for weak pointers.

@alios
Copy link

@alios alios commented on 211e221 Oct 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw if it turns out that there is an easy "fix" which makes reactive-banana compatible with the current semantics of the ghcjs runtime, it might be useful to integrated it till ghcjs and ghc have the same semantics (what should obviously the final goal).

Just for documentation, if such a patch exists, but should not be used in the native code, you could use the preprocessor to automaticly select the wanted implementation depending of compiled with native ghc or ghcjs with #if defined(ghcjs_HOST_OS) ...

@HeinrichApfelmus
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had tried to implement the workaround that Luite suggested, but apparently misread it:

The original semantics can be recovered from the modified semantics by keeping a reference to the value in the finalizer (using the touch primop for example) to explicitly keep it alive.

In line 57, the touch has to be applied to b, not to a. I'll make another attempt.

Please sign in to comment.