8317801: java/net/Socket/asyncClose/Race.java fails intermittently (aix)

[shruacha1234]

This pull request contains a backport of commit 8f121a17 from the openjdk/jdk repository.
OpenJDK bug : https://bugs.openjdk.org/browse/JDK-8317801

This fix resolves a race condition in socket close handling that led to intermittent failures in Race.java on AIX in JDK17u-dev

The original patch didn’t apply cleanly to the JDK17u-dev branch due to differences in the NIO dispatcher implementation:

• UnixDispatcher introduced in later JDK versions does not exist in JDK17.
• Virtual thread related logic in the original patch is not present in JDK17.
• The dispatcher hierarchy differs slightly.

To adapt the change for JDK17:

• NativeDispatcher.preClose was updated to accept the reader and writer thread IDs.
• Thread signalling logic was moved into FileDispatcherImpl.
• Channel implementations (SocketChannelImpl, DatagramChannelImpl, ServerSocketChannelImpl, etc.) were updated to delegate signalling through the dispatcher rather than performing it locally.

These changes preserve the functional intent of the upstream patch, ensuring that threads blocked in I/O operations are correctly signalled when a file descriptor is pre-closed.

Testing : local AIX build, java_io, java_nio, jdk_net
Additionally, executed java/net/Socket/asyncClose/Race.java 500 times to check for intermittent failures. No failures were observed.

---

Progress

• Change must not contain extraneous whitespace
• JDK-8317801 needs maintainer approval
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)

Issue

• JDK-8317801: java/net/Socket/asyncClose/Race.java fails intermittently (aix) (Bug - P4 - Rejected)

Reviewers

• Severin Gehwolf (@jerboaa - Reviewer)
• Joachim Kern (@JoKern65 - no project role)
• Thomas Stuefe (@tstuefe - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/4294/head:pull/4294
$ git checkout pull/4294

Update a local copy of the PR:
$ git checkout pull/4294
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/4294/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 4294

View PR using the GUI difftool:
$ git pr show -t 4294

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/4294.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back sacharya! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[mlbridge]

Webrevs

• 05: Full - Incremental (6bf2dffa)
• 04: Full - Incremental (84dfd5d7)
• 03: Full - Incremental (13d9c859)
• 02: Full - Incremental (8d5361ee)
• 01: Full - Incremental (b17b6bcd)
• 00: Full (9024d1ca)

[jerboaa]

This backport is being done very differently in JDK 17u because for JDK 21u the following dependencies where included first in the JDK 21 release before doing the actual backport for this bug:

• 8294399: (ch) Refactor some methods out of sun.nio.ch.UnixFileDispatcherImpl => https://bugs.openjdk.org/browse/JDK-8294399
• 8351458: (ch) Move preClose to UnixDispatcher => https://bugs.openjdk.org/browse/JDK-8351458

It would have helped if this information was provided to the reader. If we end up going this route without the enhancement backports (which should be fine IMO), then we should more accurately model it to pre-existing JDK 17u code. A review follows shortly.

[jerboaa]

/reviewers 2 Reviewer

[openjdk]

@jerboaa
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

[jerboaa]

The build failure should be resolved in a different way (rename the shared static method).

[jerboaa]

This looks OK to me. Nit: Please make static native void preClose0 in FileDispatcherImpl.java private as it's only used in that class now.

[jerboaa]

@tstuefe Could you please help with a second review for this unclean backport? Thanks!

[tstuefe]

I am not an expert in that area, and this touches shared coding, so I am a bit apprehensive about this one. Tests should definitely be done on all platforms, not only AIX. Please ask SAP, specifically @JoKern65 , for a review.

[shruacha1234]

@JoKern65 Can you please review this PR

[JoKern65]

I cannot test it on AIX at SAP, because we do not support AIX on jdk17. But the changes look good and should not introduce a different behavior on other platforms than before. And for AIX the change results in the same already established logic of the higher releases.
So for me it looks good.

[shruacha1234]

/approval JDK-8317801 request backport fix, resolves a race condition in socket close handling that led to intermittent failures in java/net/Socket/asyncClose/Race.java on AIX in 17u-dev. The upstream fix from 21u could not be applied directly due to differences in the NIO dispatcher design (absence of UnixDispatcher, related refactorings, and virtual thread support), so it was adapted by updating NativeDispatcher.preClose to take reader/writer thread IDs, moving signalling into FileDispatcherImpl, and modifying channel implementations to delegate signalling through the dispatcher. This preserves the original fix’s intent of correctly signalling threads blocked in I/O during pre-close. Tested with AIX build, java_io, java_nio, jdk_net, and 500 iterations of Race.java with no failures.

[openjdk]

@shruacha1234
JDK-8317801: The approval request has been created successfully.

[jerboaa]

This will still need a second review from an updates reviewer.

[tstuefe]

Okay, patch looks functionally equivalent to upstream patch. Thanks @JoKern65 for looking at this.

However, please run the full test suite (at least jdk tier1 and tier2) on all platforms, not only AIX, before pushing, and make sure this does not cause regressions.

@jerboaa I think we should make @JoKern65 updates reviewer. Does he have to be reviewer in mainline for that to happen?

[jerboaa]

@jerboaa I think we should make @JoKern65 updates reviewer. Does he have to be reviewer in mainline for that to happen?

Not necessarily, but it does help.

[shruacha1234]

I ran jdk:tier1 and jdk:tier2 tests on AIX, Linux, and macOS, and observed no regressions. The reported failures are the same both with and without my fix.