Skip to content

fix(transport): guard void closePromise in NativeTlsSession second-close fan-out - #63

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-transport-guard-void-closepromise-in-nativetls-d36b45
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-transport-guard-void-closepromise-in-nativetls-d36b45

Conversation

@detail-app

@detail-app detail-app Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Detail bug report: View on Detail

Change

JVM transport layer (packages/netty) — NativeTlsSession.requestClose.

  • Bug: When a peer sends TLS close_notify with half-closure disabled, NativeStreamChannel.tlsPeerClosed() initiates a close via unsafe().close(voidPromise()). That void promise flows through deferClose into NativeTlsSession.requestClose and is stored as closePromise. If any subsequent close(promise) arrives while that TLS close is still in flight, requestClose fans out by calling closePromise.addListener(...). Under the pinned Netty 4.2.18, VoidChannelPromise.addListener throws IllegalStateException("void future") unconditionally, corrupting the second caller's promise with a bogus failure — or hanging it entirely if the close was queued from a non-event-loop thread.
  • Root cause: The fan-out path lacked the isVoid() guard that NativeTlsSession.close() already applies on the same closePromise field.
  • Fix: When closePromise is void, bind the second promise to channel.closeFuture() (which always completes once the TLS close exchange or the 5-second close-timeout fallback force-closes the channel) instead of calling addListener on the void promise. This mirrors Netty's own pattern for late close requests and is consistent with the existing isVoid() guard in close().

Closes #56

Validation

  • New regression test NativeTlsDoubleCloseTest: sets up a loopback TLS client/server on the native transport, strands the close-record write (widening the in-flight window deterministically), then issues close(voidPromise()) followed by close(realPromise) from the event loop. Asserts the second promise completes successfully and closeFuture succeeds. Verified the test fails without the fix (reproducing the exact IllegalStateException: void future stack from the bug report) and passes with the fix.
  • Existing transport tests — NativeTlsClosePromiseTest, NativeReentrantCloseTest, NativeTlsOrderingTest, NativeTlsChannelTest, NativeTlsNegativeTest, NativeLifecycleTest, NativeSslEngineTest, and the full make test-jvm contract suite — all pass with no regressions. (NativeSslInteropTest failed due to /usr/bin/openssl not being executable in this environment; unrelated to this change.)
  • Build, lint, and static analysis: make build-jvm (javac --release 22 -Xlint:all -Werror), google-java-format --dry-run --set-exit-if-changed, and tools/check_java.py (Error Prone + NullAway + Checker Framework) all pass clean.

Automatic Fixes PRs can be configured here.

@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.84%. Comparing base (26fb123) to head (6328f6c).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ava/dev/elide/bemo/transport/NativeTlsSession.java 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #63      +/-   ##
============================================
+ Coverage     86.82%   86.84%   +0.01%     
- Complexity      616      619       +3     
============================================
  Files            60       60              
  Lines         16979    16986       +7     
  Branches        490      491       +1     
============================================
+ Hits          14742    14751       +9     
+ Misses         1913     1910       -3     
- Partials        324      325       +1     
Flag Coverage Δ
jvm 57.64% <72.72%> (+0.13%) ⬆️
rust 93.02% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@codspeed

codspeed Bot commented Oct 17, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 10.46%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ 6 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 112 untouched benchmarks
⏩ 65 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation retain-drop[1048576] 300.8 ns 354.9 ns -15.26%
❌ Simulation retain-drop[4096] 300.8 ns 354.9 ns -15.26%
❌ Simulation retain-drop[64] 300.8 ns 354.9 ns -15.26%
❌ Simulation retain-drop[65536] 300.8 ns 354.9 ns -15.26%
⚡ Simulation overwrite[64] 520.2 ns 466.1 ns +11.62%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing detail/bug-fix/fix-transport-guard-void-closepromise-in-nativetls-d36b45 (6328f6c) with main (26fb123)

Open in CodSpeed

Footnotes

  1. 65 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

This branch has not been deployed

No deployments
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.

[Detail Bug] Netty TLS: Second close promise fails/hangs when peer-initiated close uses void promise

1 participant