Use ResourceContexts.jl for resource handling #12
Conversation
src/DataSets.jl
Outdated
| # This is pretty delicate — it won't work if the compiler | ||
| # retains a reference to `x` due to its existence in local | ||
| # scope. | ||
| # | ||
| # Likely, it depends on | ||
| # - inlining the current closure into open() | ||
| # - Assuming that the implementation of open() itself | ||
| # doesn't keep a reference to x. | ||
| # - Assuming that the compiler (or interpreter) doesn't | ||
| # keep any stray references elsewhere. | ||
| # | ||
| # In all, it seems like this can't be reliable in general. |
There was a problem hiding this comment.
@StefanKarpinski this is the technical reason that attempting to use finalizers to turn the existing do-based API into a finalizer based one won't work. This could certainly be hacked around by a breaking change to the DataSets storage driver API and converting some structs to be mutable in order to attach finalizers.
But more generally, there's a need to have "a place to hold references to resources" which can't be solved by finalizers and are solved by passing a resource context.
The trick here is to root the resource context within the finalizer. It only works for mutable objects, or objects with mutable fields, where a finalizer can be attached by proxy. But that's already pretty good.
c42f
force-pushed
the
cjf/context-resource-handling
branch
from
8ab21c9 to
7ac2700
Compare
Of course immutable struct can't be finalized, but also there's no way around this; despite initial apperances, _root_resource is semantically unsound in general. So just bite the bullet and make Blob and BlobTree mutable. Uses the new ResourceContexts.detach_context_cleanup to transform context-based resource handling into finalizer-based resource handling.
c42f
changed the title
Updated to be more explicit the role of the unscoped vs scoped forms of open.
|
To record some slack discussion — we decided that ResourceContexts.jl is too experimental to be the main API for DataSets, but I figured out how to use it as a backend for managing resources needed by unscoped Likely ResourceContexts.jl will become part of the API for storage backends, but it's a strictly optional API for users (currently not even mentioned in the docs). |
This would use the context passing mechanism in https://github.com/c42f/ResourceContexts.jl to manage resources.
This allows datasets to be opened in the REPL without risking their underlying resource being freed, thereby fixing #8. The existing
open()-based API is retained but made safe by attaching finalizers to the returned types (as a result,BlobandBlobTreehad to become mutable which was unfortunate).In addition, the API from ResourceContexts.jl can be used, though this is kept as an implementation detail for now. This is how it looks to open a resource in the REPL using ResourceContexts.jl:
TODO