CryoThrust opened a new issue, #12342: URL: https://github.com/apache/seatunnel/issues/12342
## Description Follow-up from #12156 review (F2). Not a regression report; the race is pre-existing. Verified at `dev` commit `4c25d9a62c68f9a7e13e404c1a81b3a3c56886de` (the #12156 head, after that PR replaced the ordinal range checks): - `PhysicalPlan.cancelJob()` and `PhysicalPlan.stopJob()` evaluate `NOT_STARTED_STATUSES.contains(jobStatus)` and then call `updateJobState(...)` as two separate steps. - `updateJobState` (`PhysicalPlan.java:323`) is `synchronized` on the `PhysicalPlan` instance, and so are `startJob()` (`:379`) and `stateProcess()` (`:391`). - The not-started check is *outside* that monitor, so a job can move from `PENDING`/`CREATED`/`INITIALIZING` to `SCHEDULED`/`RUNNING` between the check and the transition. In that window `cancelJob()` would take the `CANCELED` shortcut for a job that has just started, skipping the `CANCELING` drain path. ## Why it was not folded into #12156 Moving the check under the `updateJobState` monitor changes lock ordering with `startJob()`/`stateProcess()`, rather than just relocating a condition. That deserves its own review, so #12156 deliberately kept its diff to the ordinal-check replacement plus tests and documentation. ## Expected outcome - Decide whether the not-started check should be evaluated under the same monitor as the transition it guards (a `synchronized` helper on `PhysicalPlan`, or a single method owning both steps), or whether the race is unreachable in practice — in which case it should be stated why, with a test that pins it. - If the check moves under the monitor, keep the `NOT_STARTED_STATUSES` semantics from #12156 unchanged: a job in `INITIALIZING`/`CREATED`/`PENDING` goes straight to `CANCELED` with no `CANCELING` transition. - Add regression coverage for the interleaving, not just the two static outcomes: the existing tests in `StateTransitionCleanupTest` assert the classification for a quiescent job and cannot observe a check-then-act race. ## Related - PR: #12156 (adds `NOT_STARTED_STATUSES`, `PhysicalPlan`-level cancel tests) - Review thread: #12156 comment on F2 -- 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]
