He-Pin opened a new pull request, #3740: URL: https://github.com/apache/pekko/pull/3740
### Motivation Cancelling the transport (ciphertext) output of a TLS operator tears the session down after at most one inbound round: `doOutbound` moves straight to the completed phase, and `pumpFinished()` then cancels the transport input. Inbound records the operator had already received from the peer but had not decrypted yet were discarded while the plaintext output saw a clean `onComplete`, so an application could not tell that received data had been abandoned. Every other teardown path already has an observable contract: - transport input truncated without `close_notify`: received data is delivered and `SessionTruncated` is emitted; - transport input failure: the stage fails via `failTls`; - plaintext output cancellation: accepted plaintext is drained (#3727, #3731, #3738). Reproduced on both engines and both protocols with an established session: two inbound records buffered while the plaintext output has no demand, then the transport output cancelled. Only the first record was delivered; the second was dropped with no signal at all. The control, with the transport output left open, delivers both. This came out of auditing #3739. The scenario that issue describes does not reproduce: a single buffered record is delivered once plaintext demand arrives, and the stage does not complete early. The same teardown path does lose data once more than one record is buffered. ### Modification - `TlsGraphStage`: emit `SessionTruncated` from the cancelled-transport-output branch before moving to `CompletedPhase`, so `pumpFinished()` still cancels the inputs and only `completeStage()` is deferred until the report has been pushed. Skipped when the session was never established, the inbound side is already done, or the plaintext output is closed or already holds a pending element. - `TLSActor`: the same contract through a `signallingInboundTruncation` phase gated on plaintext output demand or closure. The read side is cancelled before the pump can park there, otherwise a still-subscribed transport input would keep buffering until `Input buffer overrun` killed the actor. Reuses `OutputBunch.demandAvailableFor` instead of restating it. - Document the contract in the stream IO guide. ### Result Both engines still deliver the inbound data they can decrypt in the round that observes the cancellation, and now report `SessionTruncated` before completing instead of discarding the rest silently. Teardown timing is unchanged when the plaintext output is already closed, so the three existing `cipherOut.cancel()` cases in `TlsGraphStageIsolatedSpec`, which all cancel `plainOut` first, are unaffected. ### Tests - `stream-tests/Test/testOnly org.apache.pekko.stream.io.TlsTransportOutputCancelSpec` — 18/18 pass. Without the production change 14 of them fail and only the 4 control cases pass, so the spec is directional. It covers two buffered records plus cancellation, one buffered record plus cancellation, three buffered records drained one demand unit at a time, the no-cancellation control, and a legacy-actor case pinning that the read side is cancelled while the report waits for plaintext demand. - `stream-tests/Test/testOnly *Tls*` — 308 tests, 0 failures. - `stream/mimaReportBinaryIssues` — no issues. - `docs/paradox` — success. - `headerCheckAll` — success. - `scalafmt` 3.11.5 `--test` clean on the changed files; `git diff --check` clean. - Full suite left to CI. ### References Refs #3739 -- 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]
