BugFix: Close all FDBFileHandle on TocStore destruction#228
BugFix: Close all FDBFileHandle on TocStore destruction#228
Conversation
https://jira.ecmwf.int/browse/ECKIT-670 is refering to the issue. The main issue is that FDBFileHandles weren't closed and leaked across several executions of tests with a shared, long-living FDB.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #228 +/- ##
===========================================
+ Coverage 73.32% 73.38% +0.06%
===========================================
Files 363 363
Lines 21976 21976
Branches 2259 2259
===========================================
+ Hits 16113 16128 +15
+ Misses 5863 5848 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm not sure I like this fix. The close() function exists, because we moved close-related functionality out of the constructor. This is because any exceptions thrown in a destructor will cause std::terminate if we happen to hit he destructor due to stack unwinding from another exception being thrown. This is particularly important not in the TocStore, but in the Remote Store funtionality where close() is a non-trivial operation. In all contexts, a Store should be being owned by something. This strongly suggests that our ownership semantics are wrong. Can we check the locations where stores are being owned, and make sure that the plumbing is called correctly. I would feel much more comfortable with as ASSERT, or at the very least Log::warn() output being written, if the destructor is hit before close() is called. |
Description
FDBFileHandles weren't closed and leaked across several executions of tests with a shared, long-living FDB object.
See ECKIT-670.
Contributor Declaration
By opening this pull request, I affirm the following:
🌈🌦️📖🚧 Documentation Z3FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/z3fdb/pull-requests/PR-228
🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/fdb/pull-requests/PR-228