You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2026/10/13 04:07:55 UTC
[PR] fix: stop handing a reused ForkJoinTask to the fork-join dispatcher [pekko]
He-Pin opened a new pull request, #3621:
URL: https://github.com/apache/pekko/pull/3621
### Motivation
`Mailbox` extended `ForkJoinTask[Unit]` and was resubmitted as the *same object* for the whole lifetime of its actor (`Dispatcher.scala:151`, and the self-resubmission in `Mailbox.run()`'s `finally`). `PekkoForkJoinPool.execute` passed any `ForkJoinTask` through unwrapped:
```scala
super.execute((if (r.isInstanceOf[ForkJoinTask[?]]) r else new PekkoForkJoinTask(r)).asInstanceOf[ForkJoinTask[Any]])
```
`Mailbox` was the only object in the tree that could hit that branch — `PekkoForkJoinTask` is freshly allocated per submission and `TaskInvocation` is not a `ForkJoinTask`. So the pool pushed one long-lived reference into the same work-queue slot over and over.
That produces two independent failures.
**1. A permanently damaged work queue.** The steal path CASes on slot *content* and then writes an *absolute* `base = b + 1`. A worker that stalls between its `base` recheck and its CAS can match a later copy of the same object, several laps around the array, and move `base` backwards by a multiple of the array length. The queue then reports more tasks than its array holds while every slot is null, and nothing repairs it. On JDK 22+ every submission to that queue fails with `RejectedExecutionException: Queue capacity exceeded` until the JVM restarts; on JDK 21 and earlier tasks are silently lost or overwritten. This is exactly [ben-manes/caffeine#2021](https://github.com/ben-manes/caffeine/issues/2021), reached through Caffeine's reused `PerformCleanupTask`, and it is the trigger behind #3613.
**2. A mailbox that never runs again.** `Mailbox.exec()` rethrew whatever escaped `run()`. `ForkJoinTask.doExec()` records that via `setExceptionalCompletion`, driving `status` negative, and `doExec()` begins with `if ((s = status) >= 0)` — so every later submission of that same object returned immediately without calling `Mailbox.run()`. Nothing ever called `reinitialize()`. Tracked as #3620; both of its triggers (a fatal `Error` escaping message handling, and a `RejectedExecutionException` from the self-resubmission in `Mailbox.run()`'s `finally`) are produced by #3613.
### Modification
Following Caffeine's fix in [998978c](https://github.com/ben-manes/caffeine/commit/998978c04bfd5e2183581627283b76ceffcd70c4):
- `Mailbox` no longer extends `ForkJoinTask[Unit]`; `getRawResult`/`setRawResult`/`exec` are removed. `run()` is unchanged.
- `PekkoForkJoinPool.execute` wraps every submission in a fresh `PekkoForkJoinTask`, so the pool never sees a caller-owned task object twice. The wrapper absorbs an exceptional completion and is discarded.
- MiMa filters for the removed supertype and the three removed methods, under `2.0.x.backwards.excludes`.
`PekkoForkJoinTask.exec()` had the same `InterruptedException` / uncaught-exception-handler / rethrow behaviour as the removed `Mailbox.exec()`, so exception reporting is unchanged.
### Result
A mailbox object never returns to a work-queue slot, so dispatcher submissions cannot trigger the steal race on any JDK, and a `Throwable` escaping a mailbox run no longer disables the actor. Because `Mailbox` is no longer a `ForkJoinTask`, this also covers a user-supplied `ExecutorServiceConfigurator` returning a raw `ForkJoinPool`, which fixing only `PekkoForkJoinPool.execute` would not.
Cost is one 24-byte young-gen wrapper per mailbox *run* — not per message, since `throughput = 5` — an allocation the pool already made for every `TaskInvocation`. Each `Mailbox` instance is 8 bytes smaller, having dropped `ForkJoinTask`'s `status` and `aux` fields. Caffeine measured the equivalent change on JDK 25.0.1 (`GetPutBenchmark` plus a `weakKeys()` copy, 3 forks, `-prof gc`) and found throughput within the 99.9% confidence interval and allocation up by at most 0.06 B/op; Pekko's own `bench-jmh` ForkJoin benchmarks have not been re-run here.
`reinitialize()` before each submission, and rotating a small set of task objects, do not prevent the race — it needs only the same reference in the same slot, with any task status.
### Tests
- `sbt "actor-tests/Test/testOnly org.apache.pekko.dispatch.MailboxTaskSubmissionSpec"` — 3 passed
- The new spec fails against the pre-fix source: **2 failed, 1 passed**. The `Mailbox` type assertion and the reused-`ForkJoinTask` assertion both go red; the plain-`Runnable` control passes either way, as intended.
- `sbt "actor-tests/Test/testOnly *Dispatch* *Mailbox* *ForkJoin* *ActorSystem*"` — 188 passed, 27 suites
- `sbt "actor-tests/test"` — one flaky failure in `TcpConnectionSpec` ("send incoming data to the connection handler", 3s `expectMsgClass` timeout); the spec passes 35/35 when re-run alone
- `sbt "++2.13.18!" mimaReportBinaryIssues` — no issues (baseline `pekko-actor_2.13:1.0.0`)
- `sbt "++3.3.8!" actor/mimaReportBinaryIssues` — no issues; the same filter file covers both
- `sbt checkCodeStyle` — passed
- `scalafmt --mode diff-ref=origin/main` — no changes
Neither race can be forced from a test, so `MailboxTaskSubmissionSpec` pins the two properties that make them unreachable rather than attempting to reproduce them — the same approach Caffeine took with `scheduleDrainBuffers_notForkJoinTask`.
### Notes
This removes the *trigger* for #3613 but not that issue's two dispatcher-side defects: a double rejection still leaves a non-empty mailbox un-scheduled, and the rejection-reporting path still re-enters `dispatch` through `EventStream.publish` and can recurse into `StackOverflowError`. Those need their own changes; #3613 stays open.
`task-peeking-mode` is worth a separate look: #2870 lists changing it from `FIFO` to `LIFO` in `stream-testkit/src/test/resources/reference.conf` as an applied mitigation, but in the current tree it only appears in `actor/src/main/resources/reference.conf:490`.
### References
Refs #3613, Fixes #3620
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: stop handing a reused ForkJoinTask to the fork-join dispatcher [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3621:
URL: https://github.com/apache/pekko/pull/3621#issuecomment-6145530344
Will back port this to 1.x too
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: stop handing a reused ForkJoinTask to the fork-join dispatcher [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #3621:
URL: https://github.com/apache/pekko/pull/3621
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: stop handing a reused ForkJoinTask to the fork-join dispatcher [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #3621:
URL: https://github.com/apache/pekko/pull/3621#issuecomment-6145912447
@He-Pin can you modify the back port to not have the mina issue. I agree with the 2.0 fix but 1.x fix probably doesn't need to remove the subclassing of the FJP class.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: stop handing a reused ForkJoinTask to the fork-join dispatcher [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3621:
URL: https://github.com/apache/pekko/pull/3621#issuecomment-6145944335
Yes, I'm backporting this one , stay turn.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org