He-Pin opened a new pull request, #3264:
URL: https://github.com/apache/pekko/pull/3264
### Motivation
`Timers` rewrites `actorCell.currentMessage` to the **unwrapped** timer
message
before invoking the user's `receive`, so that `stash()` (which reads
`currentMessage` directly) captures the actual message rather than the
internal
`TimerMsg` wrapper.
That rewrite was guarded by `this.isInstanceOf[Stash]`. But `Stash`,
`UnboundedStash` and `UnrestrictedStash` are **siblings** — they all extend
`UnrestrictedStash` / `StashSupport`, and
`UnboundedStash`/`UnrestrictedStash`
are **not** subtypes of `Stash`. So for an actor mixing in `UnboundedStash`
or
`UnrestrictedStash` directly, the guard was `false`, the rewrite was skipped,
and `stash()` captured the `TimerMsg` wrapper. On `unstashAll()` the wrapper
was
intercepted again and discarded (the single-shot timer was already removed,
or
the generation was stale for a periodic timer), so **the message was lost**.
The same guard also excluded the Java‑API stash actors
(`AbstractActorWithStash`
and friends, via `AbstractActorStashSupport`), which have the same problem.
Fixes #3258.
### Modification
- Guard the `currentMessage` rewrite with `this.isInstanceOf[StashSupport]`
instead of `this.isInstanceOf[Stash]`. `StashSupport` is the common base of
all stash‑capable actors (`Stash`, `UnboundedStash`, `UnrestrictedStash`,
and
the Java‑API stash classes), and it is exactly the set of actors whose
`stash()` reads `currentMessage` — so it is the precise, correct boundary.
- Expanded the explanatory comment so the sibling relationship and the
re‑stash/lost‑message mechanism are clear (keeping the original `#24557`
reference).
### Result
- Timer messages stashed with `UnboundedStash` or `UnrestrictedStash` (and
the
Java‑API stash actors) are no longer lost on `unstashAll()`.
- Behavior for `Stash` is unchanged, and non‑stashing actors are unaffected
(the guard is still `false` for them).
- The change is a method‑body change in `Timers.aroundReceive`; **no public
API
or binary‑compatibility change** (`actor / mimaReportBinaryIssues` is
clean).
### Tests
- `sbt "actor-tests/testOnly org.apache.pekko.actor.TimerSpec
org.apache.pekko.actor.TimersAndStashSpec"`
→ all passed.
- Added directional tests mirroring the existing `Stash` test, for
`UnboundedStash` and `UnrestrictedStash` (the latter with an explicit
deque‑based mailbox, since `UnrestrictedStash` does not declare
`RequiresMessageQueue`). Both **fail before the fix** (the stashed timer
message is lost: `expectMsg("scheduled")` times out) and pass after.
- `sbt "actor/mimaReportBinaryIssues"` → no binary issues.
### References
Fixes #3258
---
This is an original contribution to Apache Pekko, made under the Apache
License
2.0 (i.e. the changes are now Apache licensed). No third-party or
Akka-derived
code is included.
--
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]
