dosubot[bot] commented on issue #3033: URL: https://github.com/apache/hugegraph/issues/3033#issuecomment-4496941254
<!-- Answer --> Here's the edited version of your issue with the requested additions and light rephrasing: *** ## TL;DR **Component:** `CachedGraphTransaction` and `CachedSchemaTransaction` (cache layer, store event listener lifecycle) **Bug:** When multiple `CachedGraphTransaction` instances share a graph and a non-owner transaction closes first, the shared store event listener is orphaned — it remains registered with the backend provider but becomes untracked, so it is never cleaned up. **Impact:** On long-running HugeGraph servers, each leaked listener holds references to cache structures and continues receiving store events indefinitely, resulting in a slow memory leak and unnecessary event processing that accumulates over the JVM lifetime. *** ## Context HugeGraph's cached transaction layer registers *store event listeners* with the backend provider to perform **bulk cache invalidation** when critical store lifecycle events occur (`STORE_INIT`, `STORE_CLEAR`, `STORE_TRUNCATE`) [[1]](https://github.com/apache/hugegraph/blob/9126c80e414c073b60fc074361b211cfb409e0eb/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java#L123-L135). These listeners ensure that in-memory vertex/edge/schema caches are wiped when the underlying store state is fundamentally altered. Because the cache is a singleton per `spaceGraphName`, the code uses a static `storeEventListenStatus` map to ensure only one listener is registered per graph, regardless of how many transaction instances exist [[2]](https://github.com/apache/hugegraph/blob/9126c80e414c073b60fc074361b211cfb409e0eb/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java#L136-L138). The first transaction to call `listenChanges()` becomes the implicit "owner" — it registers the listener with `store().provider().listen(...)`. Subsequent transactions see the map entry and skip registration. The bug is in the teardown path: any transaction — owner or not — can remove the tracking entry and break the unregistration contract. *** ## Expected behavior When several `CachedGraphTransaction` instances share the same graph, closing a non-owner transaction must not disturb the shared store listener registered by the owner. The store listener should only be unregistered when the last interested transaction closes (or, ideally, when the graph itself is disposed). ## Actual behavior `unlistenChanges()` calls `storeEventListenStatus.remove(graphName)` unconditionally [[3]](https://github.com/apache/hugegraph/blob/9126c80e414c073b60fc074361b211cfb409e0eb/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java#L193-L202). Whichever transaction closes first wins the remove. If it is a non-owner: 1. It pops the tracking entry successfully. 2. It calls `provider().unlisten(this.storeEventListener)` on its own per-instance listener — which was never registered — so the call is a no-op. 3. The owner's real store listener stays registered but is now untracked. 4. When the owner closes later, `remove(graphName)` returns `null`, the real `unlisten(...)` call is skipped, and the listener leaks for the rest of the JVM lifetime. The same pattern is present in `CachedSchemaTransaction` [[4]](https://github.com/apache/hugegraph/blob/9126c80e414c073b60fc074361b211cfb409e0eb/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedSchemaTransaction.java#L140-L147). ## Reproduction steps 1. Open two `CachedGraphTransaction` instances — `T1` (owner) and `T2` (non-owner) — for the same `spaceGraphName`. 2. `T1.listenChanges()` registers `T1.storeEventListener` via `store().provider().listen(...)` and inserts the tracking entry. 3. `T2.listenChanges()` sees the entry already present and skips registration. 4. Close `T2` first — `storeEventListenStatus.remove(graphName)` returns `true`; `provider().unlisten(T2.storeEventListener)` runs as a no-op (never registered). 5. Close `T1` — `storeEventListenStatus.remove(graphName)` returns `null`; the real `provider().unlisten(T1.storeEventListener)` is skipped. 6. `T1.storeEventListener` stays registered with the provider indefinitely. ## Root cause This is the same owner-first-close shape that [PR #3017](https://github.com/apache/hugegraph/pull/3017) fixed for `graphCacheEventListeners` via the ref-counted `CacheListenerHolder` [[5]](https://github.com/apache/hugegraph/pull/3017). `storeEventListenStatus` was left as-is, with the gap explicitly flagged in a TODO comment added by that PR: > *"storeEventListenStatus has the same owner-first close bug this PR fixes for GRAPH_CACHE_EVENT_LISTENERS."* Relevant code locations: - [`CachedGraphTransaction` unlistenChanges](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/core/CachedGraphTransaction.java#L238-L246) - Mirror in [`CachedSchemaTransaction`](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/core/CachedSchemaTransaction.java) ## Suggested fix Two options surfaced in the PR #3017 review: 1. **Ref-counted holder** — Apply the same `CacheListenerHolder` pattern used for the cache listener to the store listener. Smaller mechanical change; proven pattern already in-tree. 2. **Move lifetime to graph open/dispose** — The cache is already a `CacheManager` singleton per `spaceGraphName`, so the store listener's natural lifetime is the graph, not the transaction. Register on graph open, unregister on graph dispose. This eliminates the owner concept entirely and removes the whole race class. Option 2 is preferred for long-term cleanliness; Option 1 is the safer incremental fix. ## Scope - Apply the chosen fix to both `CachedGraphTransaction` and `CachedSchemaTransaction`. - Remove any test workarounds (e.g. `putIfAbsent` re-insertion after closing a secondary transaction) from `CachedGraphTransactionTest`. - Add a regression test exercising non-owner-first-close ordering, asserting the owner's listener is properly unregistered when the owner closes. ## References - PR #3017 review thread on `CachedGraphTransactionTest.java:236` (TODO follow-up note) - PR #3017 review thread on `CachedGraphTransaction.java:210` (owner-first-close discussion) - Introduced as a non-blocking TODO in PR #3017 (cache event alignment) <!-- Dosu Comment Footer --> *To reply, just mention [@dosu](https://go.dosu.dev/dosubot?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=reply-with-mention&utm_term=apache%2Fhugegraph).* --- Share context across your team and agents. Try [Dosu](https://dosu.dev?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=knowledge-infrastructure-tagline&utm_term=apache%2Fhugegraph). [](https://app.dosu.dev/response-feedback?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=knowledge-infrastructure-feedback&utm_term=apache%2Fhugegraph&message_id=ecc14105-4273-492f-af50-3d9890f9c3bc) [](https://github.dosu.com/apache/hugegraph?utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=knowledge-infrastructure-learn-repo&utm_term=apache%2Fhugegraph) [](https://app.dosu.dev/signup?referrer=openSource&source=github-footer&utm_source=github&utm_medium=bot-comment&utm_campaign=github-comment-footer-20260415&utm_content=knowledge-infrastructure-add-team&utm_term=apache%2Fhugegraph) -- 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]
