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

Introduce the bracketWithRegistry combinator #2892

Closed
nfrisby opened this issue Jan 22, 2021 · 1 comment · Fixed by #2893
Closed

Introduce the bracketWithRegistry combinator #2892

nfrisby opened this issue Jan 22, 2021 · 1 comment · Fixed by #2893
Assignees
Labels
consensus issues related to ouroboros-consensus technical debt

Comments

@nfrisby
Copy link
Contributor

nfrisby commented Jan 22, 2021

During the post mortem for Issue #2870, we invented an idea to help prevent it in the future. That bug was due to confusion caused by the fact that we allocated a registry and also a narrower-manager (a ChainDB Reader/Follower) that in turn used that registry, but did not register itself in that registry, despite the registry and the manager having the same lifetime. We incorrectly assumed that newReader registered itself in the registry that it was passed.

So we're going to introduce a ResourceRegistry combinator for use in this situation, where a bracket-ed thing and a registry it needs share the same lifetime. This will let us be very explicit about that coincident lifetime, which should make it very obvious if future changes cause the lifetimes to become different.

@nfrisby
Copy link
Contributor Author

nfrisby commented Jan 27, 2021

The only lingering thought is that the interface we chose for the combinator uses ResourceRegistry m -> m a instead of m (ResourceRegistry m -> a). The former is usually the first we think of and is more flexible, but the latter restricts the usage in a subtle way that seems more precise for our intended use of the combinator. The key idea is that the former lets the allocator of a allocate other resources in the registry. That's not necessarily problematic, but in the context of this function, it shouldn't be necessary to put any such resources in the registry: they can be handled in the bracketing logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant