He-Pin commented on issue #3739:
URL: https://github.com/apache/pekko/issues/3739#issuecomment-6529310326
I re-checked this on `main` (f785e2fe74) with probe-level tests against both
engines. The scenario as described does not reproduce, so I am closing this.
The reasoning is below so it stays auditable, and the related defect I did find
on the same teardown path is fixed in #3740.
### What I measured
Harness: the operator under test with all four ports on test probes, driven
by a plain `SSLEngine` peer. The session is established first, then the peer
sends an application record while the plaintext output has **no** outstanding
demand, then the transport (ciphertext) output is cancelled.
| engine | protocol | closing | completes before plaintext demand? | record
delivered once demand arrives? |
| --- | --- | --- | --- | --- |
| graph-stage | TLSv1.3 | IgnoreComplete | no | yes,
`SessionBytes("response")` |
| graph-stage | TLSv1.3 | EagerClose | no | yes |
| legacy-actor | TLSv1.3 | IgnoreComplete | no | yes |
| legacy-actor | TLSv1.3 | EagerClose | no | yes |
| graph-stage | TLSv1.2 | IgnoreComplete | no | yes |
| legacy-actor | TLSv1.2 | IgnoreComplete | no | yes |
All six then complete normally and cancel the transport input.
### Where the internal note goes wrong
> A cancelled transport output makes `outboundCompleted` (and thus
`bidirectionalCompleted`) true immediately, so the pump skips every phase and
reaches `pumpFinished`, which cancels the transport input queue
`bidirectionalCompleted` is `outboundCompleted && inboundCompleted`. A
cancelled transport output does make `outboundCompleted` true, but
`inboundCompleted` additionally requires `(transportInput.isCompleted ||
userOutput.isClosed) && (engine.isInboundDone || userOutput.isErrored)`. With
the transport input still open and the engine's inbound side not done, that is
false, so the bidirectional phase is neither executable nor completed: the pump
idles instead of reaching `pumpFinished`, and the transport input queue is not
cancelled. When plaintext demand arrives, `inboundReady` becomes true, the
record is unwrapped and pushed, and only then does `doOutbound` observe the
cancelled transport output and move to `CompletedPhase`.
The legacy actor behaves the same way: `inbound`'s `isCompleted` needs
`userOutCancelled.isCompleted`, i.e. `engine.isInboundDone ||
outputBunch.isErrored(UserOut)`, so `bidirectional` does not complete on a
transport output cancellation either.
Steps 1 and 2 of the scenario also cannot both hold here. Inbound decryption
is gated on plaintext output demand (`inboundReady` requires
`userOutput.isDemandAvailable`; the actor requires
`demandAvailableFor(UserOut)`), so a session can only be established if the
plaintext output had demand at some point. With a plaintext output that never
requested anything, the handshake itself never progresses — I hit exactly that
while building the harness.
### The related defect that is real
Auditing the same teardown path did turn up a genuine loss, narrower than
the one described: when **two or more** inbound transport elements are buffered
while the plaintext output has no demand, cancelling the transport output
delivers only the first and discards the rest with a clean `onComplete` and no
`SessionTruncated`. Reproduced on both engines and both protocols. The control,
with the transport output left open, delivers both records.
Cause: `doOutbound` moves straight to the completed phase on a cancelled
transport output, so the pump performs at most one inbound round before
`pumpFinished()` cancels the transport input.
#3740 fixes that by reporting `SessionTruncated` on this path in both
engines, with a directional spec where 14 of 18 cases fail without the
production change.
--
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]
