[C++] Fix ManagedQuery segv: ctor/dtor order fix
#3544
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3544 +/- ##
==========================================
+ Coverage 86.27% 86.32% +0.04%
==========================================
Files 55 55
Lines 6369 6369
==========================================
+ Hits 5495 5498 +3
+ Misses 874 871 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| mq = X.read().tables() | ||
| return mq | ||
|
|
||
| for _ in range(100): |
There was a problem hiding this comment.
Is this loop here because the error is intermittent?
There was a problem hiding this comment.
It is not intermittent in my setup. But Python doesn't give any real guarantees about GC behavior, so it is possible that multiple calls might be required to trip over it in some Python implementation. I was just being cautious - trying to be triple-sure we would eventually trigger GC of the reference holders
There was a problem hiding this comment.
Please leave a code comment to that effect within the test case -- otherwise future readers may be puzzled ...
johnkerl
changed the title
ManagedQuery segv: ctor/dtor order fix
* ctor/dtor order fix * lint * add comments * lint * add test case * add commnet
* ctor/dtor order fix * lint * add comments * lint * add test case * add commnet Co-authored-by: Bruce Martin <[email protected]>
Issue and/or context:
But reported via chanzuckerberg/cellxgene-census#1327 - root cause was incorrect destructor ordering in the new ManagedQuery class.
The tiledb::Array class requires a tiledb::Context to destruct, but only holds a
std::reference_wrapper<Context>. It is holder's responsibility to ensure that the Array's context is alive when the array is destructed.ManagedQuery just had the context and array shared pointers in the wrong order, causing the context to destruct first in some cases.
[sc-61614]
Changes:
Change declaration order of members, thereby correctly ordering the dtor calls.