zhztheplayer commented on code in PR #12740:
URL: https://github.com/apache/gluten/pull/12740#discussion_r3749727752
##########
cpp/core/utils/ObjectStore.cc:
##########
@@ -46,12 +46,19 @@ std::pair<gluten::ObjectStore*, gluten::ResourceHandle>
gluten::ObjectStore::loo
gluten::ObjectStore::~ObjectStore() {
for (;;) {
- if (aliveObjects_.empty()) {
- break;
- }
std::shared_ptr<void> tempObj;
{
const std::lock_guard<std::mutex> lock(mtx_);
+ // The empty-check and the pop of the last alive object must be atomic
under `mtx_`. Another
+ // thread may still call the static `ObjectStore::release()` on this
store while it is being
+ // destructed (e.g. a script-transformation feed thread closing its
output iterator during
+ // task teardown), which erases entries from `aliveObjects_` under the
lock. If the check were
+ // done outside the lock, a concurrent erase could empty the map between
the check and
+ // `rbegin()`, making `rbegin()` dereference an empty tree and crash
with a native SIGSEGV in
+ // `std::_Rb_tree_decrement`.
Review Comment:
nit: Can we keep the comment one-liner? The following can be enough:
```
// The empty-check and the pop of the last alive object must be atomic under
`mtx_`.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
