Skip to content

fix: prevent NPE in netty HandlerSubscriber - #7297

Merged
zoewangg merged 1 commit into
masterfrom
zoewang/handler-subscriber-npe-fix
Aug 20, 2026
Merged

fix: prevent NPE in netty HandlerSubscriber#7297
zoewangg merged 1 commit into
masterfrom
zoewang/handler-subscriber-npe-fix

Conversation

@zoewangg

Copy link
Copy Markdown
Contributor

Motivation and Context

Async S3 uploads (PutObject, UploadPart) on the netty-nio-client fail
intermittently under high concurrency with a non-retryable
NullPointerException:

java.lang.NullPointerException: Cannot invoke "org.reactivestreams.Subscription.request(long)" because "this.subscription" is null
    at ...nrs.HandlerSubscriber.maybeRequestMore(HandlerSubscriber.java:303)
    at ...nrs.HandlerSubscriber.channelWritabilityChanged(HandlerSubscriber.java:157)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelWritabilityChanged(...)

HandlerSubscriber is the reactive-streams Subscriber that pumps a request
body to the Netty channel. Its subscription field starts null and is only
assigned in onSubscribe. For Expect: 100-continue requests the body
subscription is deliberately deferred: HttpStreamsClientHandler holds the
subscriber aside until the server returns 100 Continue, but the
HandlerSubscriber is already in the pipeline and keeps receiving channel
events. If channelWritabilityChanged fires during that window and the channel
is writable, it calls maybeRequestMore(), which dereferences the still-null
subscription and throws. The exception surfaces as a SdkClientException
classified non-retryable, so the upload fails outright.

The S3 async client sets Expect: 100-continue on PutObject /
UploadPart, so async S3 upload takes the deferred-subscribe path; a
writability transition during the round trip is common under high-concurrency
uploads with backpressured sockets, which is why low-volume callers rarely see
it.

Fixes #7271

Modifications

channelWritabilityChanged() now calls maybeRequestMore() only when the
subscriber is RUNNING. RUNNING is reached only after onSubscribe has
assigned the subscription, so gating on it removes the null dereference while
the subscription is pending. channelWritabilityChanged() still propagates the
event down the pipeline via ctx.fireChannelWritabilityChanged() exactly as
before, and maybeRequestMore() is otherwise unchanged. Once the
100 Continue arrives and the subscribe completes, the existing
provideSubscription -> maybeStart -> maybeRequestMore path requests demand as
it does today, so no demand is lost and the upload streams normally.

Testing

Added HandlerSubscriberExpectContinueTest, a deterministic regression test in
the nrs package. It builds an EmbeddedChannel with a real
HttpStreamsClientHandler, writes a StreamedHttpRequest carrying
Expect: 100-continue, and forces a channel writability transition before the
100 Continue response is delivered. Two cases:

  • channelWritabilityChanged_whenSubscriptionPending_doesNotRouteExceptionToPipeline
    asserts the writability transition raises no exception (nothing routed to
    exceptionCaught) while the subscription is pending.
  • channelWritabilityChanged_whenSubscriptionPending_bodyStreamsAfter100Continue
    asserts that after 100 Continue arrives the body is subscribed and streamed.

With the fix reverted, both cases reproduce the exact reported NPE
(maybeRequestMore <- channelWritabilityChanged); with the fix they pass.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

A channelWritabilityChanged event during the Expect: 100-continue
subscribe-deferral window called maybeRequestMore() while the body
subscription was still null, throwing a non-retryable NPE that failed
async S3 uploads under high concurrency. Only call maybeRequestMore()
once the subscriber is RUNNING.

Fixes #7271
@zoewangg
zoewangg requested a review from a team as a code owner August 20, 2026 01:38
@zoewangg
zoewangg added this pull request to the merge queue Aug 20, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 20, 2026
@zoewangg
zoewangg enabled auto-merge August 20, 2026 20:19
@zoewangg
zoewangg added this pull request to the merge queue Aug 20, 2026
Merged via the queue into master with commit 858c1dc Aug 20, 2026
17 of 18 checks passed
@github-actions

Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

3 participants