Skip to content

Commit ee6448f

Browse files
committed
Re-enable NoThunks tests (#444)
Tests are re-enabled, but in cases where I expect failures related to `StrictMVar`s, we ignore the thunk and keep checking further into the `MVar` contents.
1 parent 31491d8 commit ee6448f

File tree

3 files changed

+36
-8
lines changed

3 files changed

+36
-8
lines changed

lsm-tree.cabal

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ library extras
320320
, contra-tracer
321321
, deepseq
322322
, fs-api
323+
, fs-sim
323324
, io-classes:strict-mvar
324325
, io-classes:strict-stm
325326
, lsm-tree

src-extras/Database/LSMTree/Extras/NoThunks.hs

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ import KMerge.Heap
6868
import NoThunks.Class
6969
import System.FS.API
7070
import System.FS.BlockIO.API
71+
import System.FS.IO
72+
import System.FS.Sim.MockFS
7173
import Test.QuickCheck (Property, Testable (..), counterexample)
7274
import Unsafe.Coerce
7375

@@ -542,7 +544,7 @@ instance (NoThunks a, Typeable s, Typeable a) => NoThunks (MutableHeap s a) wher
542544
-- a)@, can not be satisfied for arbitrary @m@\/@s@, and must be instantiated
543545
-- for a concrete @m@\/@s@, like @IO@\/@RealWorld@.
544546
class ( forall a. NoThunks a => NoThunks (StrictTVar m a)
545-
, forall a. NoThunks a => NoThunks (StrictMVar m a)
547+
, forall a. (NoThunks a, Typeable a) => NoThunks (StrictMVar m a)
546548
) => NoThunksIOLike' m s
547549

548550
instance NoThunksIOLike' IO RealWorld
@@ -564,11 +566,37 @@ instance NoThunks a => NoThunks (StrictTVar IO a) where
564566
#endif
565567
#endif
566568

567-
instance NoThunks a => NoThunks (StrictMVar IO a) where
568-
showTypeOf (_ :: Proxy (StrictMVar IO a)) = "StrictMVar IO"
569-
wNoThunks ctx var = do
570-
x <- readMVar var
571-
noThunks ctx x
569+
-- TODO: in some cases, strict-mvar functions leave thunks behind, in particular
570+
-- modifyMVarMasked and modifyMVarMasked_. So in some specific cases we evaluate
571+
-- the contents of the MVar to WHNF, and keep checking nothunks from there. See
572+
-- lsm-tree#444.
573+
--
574+
-- TODO: we tried using overlapping instances for @StrictMVar IO a@ and
575+
-- @StrictMVar IO (MergingRunState IO h)@, but the quantified constraint in
576+
-- NoThunksIOLike' will throw a compiler error telling us to mark the instances
577+
-- for StrictMVar as incoherent. Marking them as incoherent makes the tests
578+
-- fail... We are unsure if it can be overcome, but the current casting approach
579+
-- works, so there is no priority to use rewrite this code to use overlapping
580+
-- instances.
581+
instance (NoThunks a, Typeable a) => NoThunks (StrictMVar IO a) where
582+
showTypeOf (p :: Proxy (StrictMVar IO a)) = show $ typeRep p
583+
wNoThunks ctx var
584+
| Just (Proxy :: Proxy (MergingRunState IO HandleIO))
585+
<- gcast (Proxy @a)
586+
= workAroundCheck
587+
| Just (Proxy :: Proxy (MergingRunState IO HandleMock))
588+
<- gcast (Proxy @a)
589+
= workAroundCheck
590+
| otherwise
591+
= properCheck
592+
where
593+
properCheck = do
594+
x <- readMVar var
595+
noThunks ctx x
596+
597+
workAroundCheck = do
598+
!x <- readMVar var
599+
noThunks ctx x
572600

573601
{-------------------------------------------------------------------------------
574602
vector

test/Test/Database/LSMTree/Normal/StateMachine.hs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -957,8 +957,7 @@ runIO action lookUp = ReaderT $ \(session, handler) -> do
957957
x <- aux (unwrapSession session) handler action
958958
case session of
959959
WrapSession sesh ->
960-
-- TODO: Re-enable NoThunks assertions. See lsm-tree#444.
961-
const id (assertNoThunks sesh) $ pure ()
960+
assertNoThunks sesh $ pure ()
962961
pure x
963962
where
964963
aux ::

0 commit comments

Comments
 (0)