Skip to content

Commit 9a93adf

Browse files
committed
Use unsafeIOToPrimStrict in reference checks
Now that we use `unsafeIOToPrim` on a more granular level, some side effects like clearing the `globalForgottenRef` were not happening at the right time. Adding a stricter `unsafeIOToPrimStrict` version seems to help.
1 parent d974eda commit 9a93adf

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

src-control/Control/RefCount.hs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -455,8 +455,19 @@ globalRefIdSupply = unsafePerformIO $ newPrimVar 0
455455
globalForgottenRef :: IORef (Maybe (RefId, CallStack))
456456
globalForgottenRef = unsafePerformIO $ newIORef Nothing
457457

458+
-- | This version of 'unsafeIOToPrim' is strict in the result of the arument
459+
-- action.
460+
--
461+
-- Without strictness it seems that some IO side effects are not happening at
462+
-- the right time, like clearing the @globalForgottenRef@ in
463+
-- @assertNoForgottenRefs@.
464+
unsafeIOToPrimStrict :: PrimMonad m => IO a -> m a
465+
unsafeIOToPrimStrict k = do
466+
!x <- unsafeIOToPrim k
467+
pure x
468+
458469
newRefTracker :: PrimMonad m => CallStack -> m RefTracker
459-
newRefTracker allocSite = unsafeIOToPrim $ do
470+
newRefTracker allocSite = unsafeIOToPrimStrict $ do
460471
inner <- newIORef Nothing
461472
outer <- newIORef inner
462473
refid <- fetchAddInt globalRefIdSupply 1
@@ -466,7 +477,7 @@ newRefTracker allocSite = unsafeIOToPrim $ do
466477

467478
releaseRefTracker :: (HasCallStack, PrimMonad m) => Ref a -> m ()
468479
releaseRefTracker Ref { reftracker = RefTracker _refid _weak outer _ } =
469-
unsafeIOToPrim $ do
480+
unsafeIOToPrimStrict $ do
470481
inner <- readIORef outer
471482
let releaseSite = callStack
472483
writeIORef inner (Just releaseSite)
@@ -492,18 +503,22 @@ finaliserRefTracker inner refid allocSite = do
492503

493504
assertNoForgottenRefs :: (PrimMonad m, MonadThrow m) => m ()
494505
assertNoForgottenRefs = do
495-
mrefs <- unsafeIOToPrim $ readIORef globalForgottenRef
506+
mrefs <- unsafeIOToPrimStrict $ readIORef globalForgottenRef
496507
case mrefs of
497508
Nothing -> return ()
498509
Just (refid, allocSite) -> do
499510
-- Clear the var so we don't assert again.
500-
unsafeIOToPrim $ writeIORef globalForgottenRef Nothing
511+
--
512+
-- Using the strict version is important here: if @m ~ IOSim s@, then
513+
-- using the non-strict version will lead to @RefNeverReleased@
514+
-- exceptions.
515+
unsafeIOToPrimStrict $ writeIORef globalForgottenRef Nothing
501516
throwIO (RefNeverReleased refid allocSite)
502517

503518

504519
assertNoUseAfterRelease :: (PrimMonad m, MonadThrow m, HasCallStack) => Ref a -> m ()
505520
assertNoUseAfterRelease Ref { reftracker = RefTracker refid _weak outer allocSite } = do
506-
released <- unsafeIOToPrim (readIORef =<< readIORef outer)
521+
released <- unsafeIOToPrimStrict (readIORef =<< readIORef outer)
507522
case released of
508523
Nothing -> pure ()
509524
Just releaseSite -> do
@@ -517,7 +532,7 @@ assertNoUseAfterRelease Ref { reftracker = RefTracker refid _weak outer allocSit
517532

518533
assertNoDoubleRelease :: (PrimMonad m, MonadThrow m, HasCallStack) => Ref a -> m ()
519534
assertNoDoubleRelease Ref { reftracker = RefTracker refid _weak outer allocSite } = do
520-
released <- unsafeIOToPrim (readIORef =<< readIORef outer)
535+
released <- unsafeIOToPrimStrict (readIORef =<< readIORef outer)
521536
case released of
522537
Nothing -> pure ()
523538
Just releaseSite1 -> do
@@ -550,7 +565,7 @@ checkForgottenRefs = do
550565
-- Unfortunately, this relies on the implementation of the GHC scheduler,
551566
-- not on any Haskell specification, and is therefore both non-portable and
552567
-- presumably rather brittle. Therefore, for good measure, we do it twice.
553-
unsafeIOToPrim $ do
568+
unsafeIOToPrimStrict $ do
554569
performMajorGCWithBlockingIfAvailable
555570
yield
556571
performMajorGCWithBlockingIfAvailable

0 commit comments

Comments
 (0)