DanielLeens commented on PR #11727: URL: https://github.com/apache/seatunnel/pull/11727#issuecomment-5391634898
Thanks @SEZ9 — agreed on both counts, and I want to flag one thing before we wait on a fix commit: I just re-checked the PR live and the head is still `51c6a46a` (unchanged since my last comment), so there is nothing new in the diff yet for either of us to re-review. I'll hold off on a fresh full pass until an actual fix commit lands, per the plan below. On the two open items: 1. **Cross-generation eviction race** — agreed, `taskDone()` should capture `TaskGroupContext finishedContext = executionContexts.remove(taskGroupLocation);` (single-winner via the `ConcurrentHashMap` CAS) and recycle only that instance, rather than `recycleClassLoader()` re-resolving by key via its own `get()`. Key-only cleanup is unsafe precisely because `TaskGroupLocation` is reused across restore generations, which is the whole premise of this fix — so this needs to be closed here, not deferred. 2. **NPE / silent-null class loader path** — agreed, `taskGroupContext.getClassLoaders().get(t.getTaskID())` in `BlockingWorker.run()` needs a guard after the existing null-context check, with a fail-fast `IllegalStateException` (matching this PR's own style) instead of a silent null context class loader. On the regression test extension: also agreed — a companion case where a newer generation installs its context under the same key before the older generation's `taskDone()` runs would pin the exact race these two issues describe, on top of the existing `TaskDeployStaleContextRaceTest` coverage. @abdessalems — once a fix commit lands addressing both points (plus, ideally, the extended test), ping me and I'll do a fresh pass focused on just those two locations rather than re-running the full template, since the rest of the diff hasn't changed since my last approval. -- 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]
