-
Notifications
You must be signed in to change notification settings - Fork 41
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
Generalized mlocking via MonadST #404
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. 👍
cardano-crypto-class/src/Cardano/Crypto/Libsodium/Memory/Internal.hs
Outdated
Show resolved
Hide resolved
1c1fdfb
to
7374962
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found one problem, but other than that I think it is ready to be merged
cardano-crypto-class/src/Cardano/Crypto/Libsodium/Memory/Internal.hs
Outdated
Show resolved
Hide resolved
20f2fe3
to
1e67222
Compare
Changes: - Remove MonadSodium typeclass - Express mlocked memory operations in terms of MonadST - Get rid of ForgetMock testing (has been moot since we no longer depend on GC to reclaim mlocked memory) - Remove `m` parameter from mlocked memory based typeclasses (KES, DSIGN) - Simplify KES and DSIGN typeclass hierarchies
00c4e4e
to
4ee7879
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!!! Thank you for doing the work
This is an alternative approach to generalizing mlocking allocations over "arbitrary" monads (specifically, we want
IO
andIOSim
).#388 introduces a
MonadMLock
typeclass, in much the same way asio-classes
providesMonadMVar
,MonadDelay
, etc.; this PR takes a different approach, based on the observation that all the mlocked allocation and raw memory manipulation APIs, while technically inIO
, are morally inST
- their only effects are memory manipulations, there's no I/O going on, no randomness, etc. So we provide versions of all those primitives and FFI functions inST
(or rather, with aMonadST
constraint), which automatically makes them available inIO
andIOSim
.One complication of this is that we cannot use the
MonadMLock
typeclass to inject mlock allocation logging, like we did in #388; we solve this by changing the relevant APIs such that they take an explicitMLockedAllocator
argument, and API-backards-compatible convenience wrappers that default the allocator tomlockedMalloc
. And now we can implement mlocked allocation logging by passing a "decorated" allocator (which not only allocates, but also logs the allocation, and attaches deallocation logging code to the finalizer), while using the compatible wrappers outside of testing.