Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize StrictT(M)Var invariant #1040

Merged
merged 1 commit into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions io-sim-classes/io-sim-classes.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ source-repository head
location: https://github.com/input-output-hk/ouroboros-network
subdir: io-sim-classes

flag checktvarinvariant
Copy link
Contributor

Choose a reason for hiding this comment

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

When/where do we enable this flag?

Thinking out loud: we want to enable it always (?) for test-consensus and test-storage. We don't want to have to pass it manually via the cmd line.

Description: Enable runtime invariant checks on StrictT(M)Var
Manual: True
Default: False

library
hs-source-dirs: src

Expand Down Expand Up @@ -53,3 +58,5 @@ library
-Wno-unticked-promoted-constructors
-fno-ignore-asserts

if flag(checktvarinvariant)
cpp-options: -DCHECK_TVAR_INVARIANT
64 changes: 37 additions & 27 deletions io-sim-classes/src/Control/Monad/Class/MonadSTM/Strict.hs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{-# LANGUAGE BangPatterns #-}
{-# LANGUAGE CPP #-}
{-# LANGUAGE DuplicateRecordFields #-}
{-# LANGUAGE NamedFieldPuns #-}
module Control.Monad.Class.MonadSTM.Strict
Expand Down Expand Up @@ -31,7 +32,6 @@ module Control.Monad.Class.MonadSTM.Strict
, isEmptyTMVar
) where

import Control.Exception (assert)
import Control.Monad.Class.MonadSTM as X hiding (LazyTMVar, LazyTVar,
isEmptyTMVar, modifyTVar, newEmptyTMVar, newEmptyTMVarM,
newTMVar, newTMVarM, newTVar, newTVarM, putTMVar,
Expand All @@ -45,9 +45,8 @@ import GHC.Stack
-------------------------------------------------------------------------------}

data StrictTVar m a = StrictTVar
{ invariant :: !(a -> Bool)
-- ^ Invariant checked in an 'assert' whenever storing an @a@ in the
-- 'StrictTVar'.
{ invariant :: !(a -> Maybe String)
-- ^ Invariant checked whenever updating the 'StrictTVar'.
, tvar :: !(Lazy.LazyTVar m a)
}

Expand All @@ -56,25 +55,25 @@ toLazyTVar :: StrictTVar m a -> Lazy.LazyTVar m a
toLazyTVar StrictTVar { tvar } = tvar

newTVar :: MonadSTM m => a -> STM m (StrictTVar m a)
newTVar !a = StrictTVar (const True) <$> Lazy.newTVar a
newTVar !a = StrictTVar (const Nothing) <$> Lazy.newTVar a

newTVarM :: MonadSTM m => a -> m (StrictTVar m a)
newTVarM = newTVarWithInvariantM (const True)
newTVarM = newTVarWithInvariantM (const Nothing)

newTVarWithInvariantM :: MonadSTM m
=> (a -> Bool) -- ^ Invariant
newTVarWithInvariantM :: (MonadSTM m, HasCallStack)
=> (a -> Maybe String) -- ^ Invariant (expect 'Nothing')
-> a
-> m (StrictTVar m a)
newTVarWithInvariantM invariant !a =
assert (invariant a) $
checkInvariant (invariant a) $
StrictTVar invariant <$> Lazy.newTVarM a

readTVar :: MonadSTM m => StrictTVar m a -> STM m a
readTVar StrictTVar { tvar } = Lazy.readTVar tvar

writeTVar :: MonadSTM m => StrictTVar m a -> a -> STM m ()
writeTVar :: (MonadSTM m, HasCallStack) => StrictTVar m a -> a -> STM m ()
writeTVar StrictTVar { tvar, invariant } !a =
assert (invariant a) $
checkInvariant (invariant a) $
Lazy.writeTVar tvar a

modifyTVar :: MonadSTM m => StrictTVar m a -> (a -> a) -> STM m ()
Expand All @@ -92,34 +91,33 @@ updateTVar v f = do
-------------------------------------------------------------------------------}

data StrictTMVar m a = StrictTMVar
{ invariant :: !(a -> Bool)
-- ^ Used in an 'assert' to check whether the given @a@ is in normal form
-- whenever storing an @a@ in the 'StrictTMVar'.
{ invariant :: !(a -> Maybe String)
-- ^ Invariant checked whenever updating the 'StrictTMVar'.
, tmvar :: !(Lazy.LazyTMVar m a)
}

newTMVar :: MonadSTM m => a -> STM m (StrictTMVar m a)
newTMVar !a = StrictTMVar (const True) <$> Lazy.newTMVar a
newTMVar !a = StrictTMVar (const Nothing) <$> Lazy.newTMVar a

newTMVarM :: MonadSTM m => a -> m (StrictTMVar m a)
newTMVarM = newTMVarWithInvariantM (const True)
newTMVarM = newTMVarWithInvariantM (const Nothing)

newTMVarWithInvariantM :: (MonadSTM m, HasCallStack)
=> (a -> Bool) -- ^ Invariant
=> (a -> Maybe String) -- ^ Invariant (expect 'Nothing')
-> a
-> m (StrictTMVar m a)
newTMVarWithInvariantM invariant !a =
assert (invariant a) $
checkInvariant (invariant a) $
StrictTMVar invariant <$> Lazy.newTMVarM a

newEmptyTMVar :: MonadSTM m => STM m (StrictTMVar m a)
newEmptyTMVar = StrictTMVar (const True) <$> Lazy.newEmptyTMVar
newEmptyTMVar = StrictTMVar (const Nothing) <$> Lazy.newEmptyTMVar

newEmptyTMVarM :: MonadSTM m => m (StrictTMVar m a)
newEmptyTMVarM = newEmptyTMVarWithInvariantM (const True)
newEmptyTMVarM = newEmptyTMVarWithInvariantM (const Nothing)

newEmptyTMVarWithInvariantM :: MonadSTM m
=> (a -> Bool) -- ^ Invariant
=> (a -> Maybe String) -- ^ Invariant (expect 'Nothing')
-> m (StrictTMVar m a)
newEmptyTMVarWithInvariantM invariant =
StrictTMVar invariant <$> Lazy.newEmptyTMVarM
Expand All @@ -131,14 +129,14 @@ takeTMVar StrictTMVar { tmvar } = Lazy.takeTMVar tmvar
tryTakeTMVar :: MonadSTM m => StrictTMVar m a -> STM m (Maybe a)
tryTakeTMVar StrictTMVar { tmvar } = Lazy.tryTakeTMVar tmvar

putTMVar :: MonadSTM m => StrictTMVar m a -> a -> STM m ()
putTMVar :: (MonadSTM m, HasCallStack) => StrictTMVar m a -> a -> STM m ()
putTMVar StrictTMVar { tmvar, invariant } !a =
assert (invariant a) $
checkInvariant (invariant a) $
Lazy.putTMVar tmvar a

tryPutTMVar :: MonadSTM m => StrictTMVar m a -> a -> STM m Bool
tryPutTMVar :: (MonadSTM m, HasCallStack) => StrictTMVar m a -> a -> STM m Bool
tryPutTMVar StrictTMVar { tmvar, invariant } !a =
assert (invariant a) $
checkInvariant (invariant a) $
Lazy.tryPutTMVar tmvar a

readTMVar :: MonadSTM m => StrictTMVar m a -> STM m a
Expand All @@ -147,10 +145,22 @@ readTMVar StrictTMVar { tmvar } = Lazy.readTMVar tmvar
tryReadTMVar :: MonadSTM m => StrictTMVar m a -> STM m (Maybe a)
tryReadTMVar StrictTMVar { tmvar } = Lazy.tryReadTMVar tmvar

swapTMVar :: MonadSTM m => StrictTMVar m a -> a -> STM m a
swapTMVar :: (MonadSTM m, HasCallStack) => StrictTMVar m a -> a -> STM m a
swapTMVar StrictTMVar { tmvar, invariant } !a =
assert (invariant a) $
checkInvariant (invariant a) $
Lazy.swapTMVar tmvar a

isEmptyTMVar :: MonadSTM m => StrictTMVar m a -> STM m Bool
isEmptyTMVar StrictTMVar { tmvar } = Lazy.isEmptyTMVar tmvar

{-------------------------------------------------------------------------------
Dealing with invariants
-------------------------------------------------------------------------------}

checkInvariant :: HasCallStack => Maybe String -> m x -> m x
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be useful to reuse this function to check other invariants than this one? I mean instead of assert, which does not support a custom error message (or a call stack with a depth > 1).

#if CHECK_TVAR_INVARIANT
checkInvariant Nothing k = k
checkInvariant (Just err) _ = error $ "Invariant violation: " ++ err
#else
checkInvariant _err k = k
#endif
2 changes: 1 addition & 1 deletion nix/.stack.nix/io-sim-classes.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ouroboros-consensus/src/Ouroboros/Storage/ChainDB/Impl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -213,5 +213,5 @@ openDBInternal args launchBgTasks = do
blockEpoch = epochInfoEpoch (Args.cdbEpochInfo args) . blockSlot

-- TODO (#969): Re-enable this and deal with the fallout.
isNF :: forall a. a -> Bool
isNF = const True -- unsafePerformIO . isNormalForm
isNF :: forall a. a -> Maybe String
isNF = const Nothing -- unsafePerformIO . isNormalForm
1 change: 0 additions & 1 deletion ouroboros-network/ouroboros-network.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ library
build-depends: hashable >=1.2 && <1.3,
text >=1.2 && <1.3


test-suite test-network
type: exitcode-stdio-1.0
hs-source-dirs: test src
Expand Down