Copilot commented on code in PR #3740:
URL: https://github.com/apache/pekko/pull/3740#discussion_r4426882190
##########
stream/src/main/scala/org/apache/pekko/stream/impl/io/TLSActor.scala:
##########
@@ -352,6 +361,21 @@ import pekko.util.ByteString
}
}
+ /**
+ * Closing the inbound side is what reveals whether the peer's close_notify
is still missing, so
+ * the plaintext output can learn that inbound records buffered from here on
are unreachable. The
+ * read side is cancelled first because delivering that report may have to
wait for plaintext
+ * demand, and a subscribed transport input would overrun its buffer while
the pump is parked.
+ */
+ private def inboundTruncationOwed(): Boolean =
+ if (!corkUser && !engine.isInboundDone && !userOutDemand.isCompleted) {
+ inputBunch.cancel()
+ try {
+ engine.closeInbound()
+ false
+ } catch { case _: SSLException => true }
+ } else false
Review Comment:
`inboundTruncationOwed()`’s Scaladoc says the *read side* is cancelled
first, but the code calls `inputBunch.cancel()`, which (by name/usage) likely
cancels more than just the transport-read side. Either (mandatory) narrow the
cancellation to only the transport input (if that’s the intent), or (optional)
update the comment to accurately describe that all inputs are cancelled here.
##########
stream/src/main/scala/org/apache/pekko/stream/impl/io/TLSActor.scala:
##########
@@ -287,6 +289,12 @@ import pekko.util.ByteString
}
}
+ val signallingInboundTruncation = TransferPhase(userOutDemand) { () =>
+ if (tracing) log.debug("signalling SessionTruncated after TransportOut
cancellation")
+ outputBunch.enqueue(UserOut, SessionTruncated)
+ nextPhase(completedPhase)
+ }
Review Comment:
The legacy-actor truncation path unconditionally enqueues `SessionTruncated`
on `UserOut` (line 294) without guarding against an already-pending plaintext
element. The `TlsGraphStage` version explicitly checks `!userOutput.hasPending`
before enqueuing, which strongly suggests the actor path should do the same to
avoid enqueue failure or reordering/overwriting when `UserOut` already has a
pending element. Consider adding an explicit `UserOut`-pending check to
`inboundTruncationOwed()` (or before `enqueue`) and, if pending is present, let
the normal delivery path drain that element before signalling truncation.
##########
stream/src/main/scala/org/apache/pekko/stream/impl/io/TLSActor.scala:
##########
@@ -352,6 +361,21 @@ import pekko.util.ByteString
}
}
+ /**
+ * Closing the inbound side is what reveals whether the peer's close_notify
is still missing, so
+ * the plaintext output can learn that inbound records buffered from here on
are unreachable. The
+ * read side is cancelled first because delivering that report may have to
wait for plaintext
+ * demand, and a subscribed transport input would overrun its buffer while
the pump is parked.
+ */
+ private def inboundTruncationOwed(): Boolean =
+ if (!corkUser && !engine.isInboundDone && !userOutDemand.isCompleted) {
+ inputBunch.cancel()
+ try {
+ engine.closeInbound()
+ false
+ } catch { case _: SSLException => true }
+ } else false
Review Comment:
The legacy-actor truncation path unconditionally enqueues `SessionTruncated`
on `UserOut` (line 294) without guarding against an already-pending plaintext
element. The `TlsGraphStage` version explicitly checks `!userOutput.hasPending`
before enqueuing, which strongly suggests the actor path should do the same to
avoid enqueue failure or reordering/overwriting when `UserOut` already has a
pending element. Consider adding an explicit `UserOut`-pending check to
`inboundTruncationOwed()` (or before `enqueue`) and, if pending is present, let
the normal delivery path drain that element before signalling truncation.
--
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]
