Skip to content

Guard against null TimeoutsHolder in NettyConnectListener.onSuccess#2176

Merged
hyperxpro merged 1 commit into
mainfrom
fix-2172
May 10, 2026
Merged

Guard against null TimeoutsHolder in NettyConnectListener.onSuccess#2176
hyperxpro merged 1 commit into
mainfrom
fix-2172

Conversation

@hyperxpro
Copy link
Copy Markdown
Member

Motivation

NettyConnectListener.onSuccess NPEs on timeoutsHolder.setResolvedRemoteAddress(...) when a request timeout fires concurrently with a successful connect: abort() calls cancelTimeouts() (nulls the holder) before setting isDone, and per JMM the IO thread can observe holder==null while isDone==0. PR #2127 only guarded the remoteAddress parameter, not the holder.

Modification

  • Add null guard on timeoutsHolder in onSuccess — close the channel and bail out if the holder was nulled out concurrently.
  • Broaden futureIsAlreadyCancelled → futureIsAlreadyCompleted (isCancelled() → isDone()) so aborted futures don't slip past the early-out (abort()/done() set isDone, not isCancelled).
  • Release the partition-key lock on both early-exit paths.
  • Add NettyConnectListenerTest reproducing the exact NPE from the bug report.

Fixes
#2172

@hyperxpro
Copy link
Copy Markdown
Member Author

@jaredstehler PTAL

@jaredstehler
Copy link
Copy Markdown

This looks like it should do the trick! Thanks for the quick reply!

@hyperxpro hyperxpro merged commit 7ca122f into main May 10, 2026
17 checks passed
@hyperxpro hyperxpro deleted the fix-2172 branch May 10, 2026 15:20
hyperxpro added a commit that referenced this pull request May 12, 2026
…2176)

Motivation

NettyConnectListener.onSuccess NPEs on
timeoutsHolder.setResolvedRemoteAddress(...) when a request timeout
fires concurrently with a successful connect: abort() calls
cancelTimeouts() (nulls the holder) before setting isDone, and per JMM
the IO thread can observe holder==null while isDone==0. PR #2127 only
guarded the remoteAddress parameter, not the holder.

Modification

- Add null guard on timeoutsHolder in onSuccess — close the channel and
bail out if the holder was nulled out concurrently.
- Broaden futureIsAlreadyCancelled → futureIsAlreadyCompleted
(isCancelled() → isDone()) so aborted futures don't slip past the
early-out (abort()/done() set isDone, not isCancelled).
- Release the partition-key lock on both early-exit paths.
- Add NettyConnectListenerTest reproducing the exact NPE from the bug
report.

Fixes
#2172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants