He-Pin opened a new pull request, #3259:
URL: https://github.com/apache/pekko/pull/3259
### Motivation
`TcpConnectionStage.closeConnectionUpstreamFinished()` dereferences the
connection `ActorRef` without a null check on the full-close path. For an
**outbound** connection whose upstream finishes **before** the `Connected`
message arrives, `connection` is still `null` (it is only assigned when
`Connected` is received), so `connection ! Close` throws a
`NullPointerException` and fails the stage.
This is easy to hit with a source that completes eagerly: `Source.empty`
completes synchronously during materialization, i.e. before the asynchronous
`Connected` reply, so `onUpstreamFinish` runs while `connection` is `null`.
The existing null check only guarded the half-close (`ConfirmedClose`) path,
leaving the `halfClose = false` path unprotected.
Additionally, the `connecting` handler always issued `ConfirmedClose` (a
half-close) regardless of the `halfClose` setting when the upstream had
already finished before the connection was established — incorrect when
half-close is disabled, because it would keep the read side open instead of
fully tearing the connection down.
Fixes #3255.
### Modification
- Hoist a `connection eq null` guard to the top of
`closeConnectionUpstreamFinished()` so neither close branch can dereference
a null connection. When the connection is not yet established the close is
**deferred** until it is — this mirrors the structure already used by the
sibling `closeConnectionDownstreamFinished()`.
- Make the `connecting` handler honor the half-close setting when the
upstream
finished before connect: send `Close` when half-close is disabled and
`ConfirmedClose` when it is enabled.
- Added explanatory comments at both sites so the deferral/half-close
reasoning is clear to future readers.
### Result
- Outbound connections with `halfClose = false` no longer throw a
`NullPointerException` when the upstream finishes before the connection is
established.
- The early-finish close path now respects the configured half-close
semantics (`Close` for full-close, `ConfirmedClose` for half-close).
- All touched symbols are `private[stream]` / `@InternalApi` internals, so
there is **no public API or binary-compatibility change** (no MiMa filter
required).
### Tests
- `sbt "stream-tests/testOnly org.apache.pekko.stream.io.TcpSpec"` → 28
passed.
- Added a directional regression test
(`TcpSpec`: "not throw a NullPointerException when full-close is requested
and upstream finishes before the connection is established"). It **fails
with the exact `NullPointerException` before the fix** (verified by
temporarily reverting the source change) and passes after.
### References
Fixes #3255
---
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]
