bito-code-review[bot] commented on PR #43696:
URL: https://github.com/apache/superset/pull/43696#issuecomment-5471113457
<!-- Bito Reply -->
The flagged race condition is a valid concern in the current implementation.
The issue arises because `cancelTask` reads the current `tabId` from the
`getTabId()` hook at the time of cancellation, which may differ from the
`tabId` used when the task was originally submitted (e.g., due to a
duplicate-tab collision or reassignment). This mismatch causes the backend to
fail to detach the correct per-tab subscription, potentially orphaning the
original task.
The PR already implements a fix for this by capturing the `tabId` at the
time of submission (`submitTabId`) and passing it explicitly to `cancelTask`
and `cancelUnwaitedTasks`. This ensures that the cancellation request always
uses the `tabId` associated with the original subscription.
I have reviewed the changes in
`superset-frontend/src/middleware/asyncEvent.ts` and the corresponding tests in
`superset-frontend/src/middleware/asyncEvent.test.ts`, and the implementation
correctly addresses the race condition by decoupling the cancellation `tabId`
from the live hook value.
**superset-frontend/src/middleware/asyncEvent.ts**
```
// Capture the tab id once, in the same tick the 202 was received, and reuse
it
// for any later cancel/detach of these tasks. Reading it fresh at cancel
time
// could send a different id if the tab id was reassigned meanwhile (a
// duplicate-tab collision), orphaning the original per-tab subscription.
const submitTabId = getTabId();
```
--
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]
