-
Notifications
You must be signed in to change notification settings - Fork 661
Open
Milestone
Description
TursoDBFactory currently caches the returned TursoDB database instances, with the url as the cache key, but this caching policy is unsafe WRT closing.
The problem with this is that TursoDB is an AutoCloseable, an interface that declares that the object should be closed after use, but it's not safe for the user to close a TursoDB created by the factory. Consider:
- caller A requests a
TursoDB, the factory creates one and returns it - caller B request a
TursoDBfor the same URL, the factory reuses the cached instance and returns it - caller A completes its processing and closes its
TursoDB - caller B's
TursoDBis now unusable because it was closed by caller A
I see a few solutions to this problem, ordered from most desirable to least desirable:
- don't cache
TursoDBinstances. This will also make it possible to open different databases to the same file but with different flags - make
TursoDBFactory::opennon-static, and make the factoryCloseable, so that callers can create a factory, and then close it when they're done. Closing it will close all trackedTursoDBinstances. It would then be acceptable to makeTursoDBFactorynon thread-safe, since the cache would be local to the caller, and not ClassLoader-wide. - from the factory, return a reference-counting wrapper around
TursoDBthat will keep track of how many times the factory was called for the same url, and only close the underlying instance whenclose()has been called on all instances.
Option 1 is the easiest, and Options 2 and 3 would likely require some additional locking, or a different thread-safety policy.
@seonWKim tagging you on this since I think you wrote most of the bindings.
Metadata
Metadata
Assignees
Labels
No labels