8294399: (ch) Refactor some methods out of sun.nio.ch.UnixFileDispatcherImpl

[shruacha1234]

Signed-off-by: Shruthi Shruthi.Shruthi1@ibm.com

OpenJDK PR : openjdk/jdk#10434
OpenJDK bug : https://bugs.openjdk.org/browse/JDK-8294399

The original patch does not apply cleanly to JDK17u-dev, as UnixFileDispatcherImpl.java and UnixFileDispatcherImpl.c do not exist in this version.
Instead, JDK 17 uses FileDispatcherImpl.java and FileDispatcherImpl.c under the unix directory for the relevant implementation.
Therefore, the changes were applied manually to align with the JDK 17 structure.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8294399 needs maintainer approval

Issue

• JDK-8294399: (ch) Refactor some methods out of sun.nio.ch.UnixFileDispatcherImpl (Enhancement - P4 - Rejected)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3631

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back shruacha1234! 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.

[shruacha1234]

@AlanBateman Can you please review this PR

[mlbridge]

Webrevs

• 01: Full - Incremental (bab00e47)
• 00: Full (267df776)

[GoeLin]

Hi @shruacha1234
please follow the instructions in https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix
of how to backport a fix.
Explain in the description above why this is not clean.
First get a review before you apply for fix-request. Maybe you find someone else than Alan, doesn't IBM have some engineers with reviewer status in the updates project?
Add an elaborate fix-request comment.

[shruacha1234]

/approval 8294399 request the backport fix. The FileDispatcherImpl class includes native methods close0(), dup0(), and preClose0(), which are commented as:

// Shared with SocketDispatcher and DatagramDispatcher but NOT used by FileDispatcherImpl

These methods are being relocated to a new class, UnixDispatcher, which is referenced by both SocketDispatcher and DatagramDispatcher, where the methods are actually used.
Additionally, the FileDispatcherImpl class contains preClose() and dup() methods that are not invoked anywhere and can therefore be safely removed.

[openjdk]

@shruacha1234
8294399: The approval request has been created successfully.

[galderz]

The changes look fine to me. The changes that were originally UnixFileDispatcherImpl.[java/c] align with the ones in FileDispatcherImpl.[java/c].

What testing have you done to validate the changes? Once that is explained and validated I'll approve.

[shruacha1234]

@galderz
I was able to create a successful AIX local build with this patch. Also, I ran the java/net and java/nio jtreg tests, and all are passing

java/net test result

 TEST TARGETS SUMMARY
19:31:13  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
19:31:13  PASSED test targets:
19:31:13  	jdk_net_0 - Test results: passed: 805 
19:31:13  	jdk_net_1 - Test results: passed: 805 
19:31:13  
19:31:13  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
19:31:13  
19:31:13  TESTCASES RESULTS SUMMARY: passed: 1,610; failed: 0; error: 0; skipped: 0
19:31:13  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
19:31:13  ALL TESTS PASSED

java/nio test result

15:01:19  
15:01:19  TEST TARGETS SUMMARY
15:01:19  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
15:01:19  PASSED test targets:
15:01:19  	jdk_nio_0 - Test results: passed: 431 
15:01:19  	jdk_nio_1 - Test results: passed: 431 
15:01:19  
15:01:19  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
15:01:19  
15:01:19  TESTCASES RESULTS SUMMARY: passed: 862; failed: 0; error: 0; skipped: 0
15:01:19  ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
15:01:19  ALL TESTS PASSED

[shruacha1234]

@galderz
Could you please let me know if the above tests are sufficient for this change. Or any other testing to be done

[shruacha1234]

Hi @GoeLin
I wanted to backport openjdk/jdk#25817 to JDK 17, but it has dependencies on two other changes:

• 8294399: (ch) Refactor some methods out of sun.nio.ch.UnixFileDispatcherImpl jdk#10434 – introduces UnixDispatcher.java and UnixDispatcher.c in src/java.base/unix/classes/sun/nio/ch/ and src/java.base/unix/native/libnio/ch/, respectively.
• 8351458: (ch) Move preClose to UnixDispatcher jdk#23956 – includes necessary refactoring.

I successfully created a local AIX build with this PR changes and verified it by running jtreg tests for java/net and java/nio. All tests passed without issues.

Please let me know if there is anything else required for the approval

[tstuefe]

Small question, why do we backport refactoring changes?

[GoeLin]

Also, if you want to backport https://bugs.openjdk.org/browse/JDK-8317801, please first backport it to 25, then 21.
Please read https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix , it e.g. advises you to first backport to higher releases. Thanks.

[shruacha1234]

@tstuefe
You mean, I can add the required refactoring and openjdk/jdk#25817 in a single PR?

[shruacha1234]

Small question, why do we backport refactoring changes?

I’m backporting 8294399 because it introduces the UnixDispatcher class, which is a necessary foundation for other functional fixes, including the one in openjdk/jdk#25817. Although it’s a refactoring change, it helps separate platform-specific logic into a dedicated class, making the code cleaner and easier to maintain. More importantly, the fix that address real issues (async close handling and race conditions) are built on top of this refactoring. Without backporting 8294399, those fixes cannot be cleanly or safely applied to JDK 17.

[shruacha1234]

@GoeLin Can you please let me know if we can port this backport PR with the below justification?

I’m backporting 8294399 because it introduces the UnixDispatcher class, which is a necessary foundation for other functional fixes, including the one in openjdk/jdk#25817. Although it’s a refactoring change, it helps separate platform-specific logic into a dedicated class, making the code cleaner and easier to maintain. More importantly, the fix that address real issues (async close handling and race conditions) are built on top of this refactoring. Without backporting 8294399, those fixes cannot be cleanly or safely applied to JDK 17.

[shruacha1234]

Also, if you want to backport https://bugs.openjdk.org/browse/JDK-8317801, please first backport it to 25, then 21. Please read https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix , it e.g. advises you to first backport to higher releases. Thanks.

I have backported the fix to JDK25. JDK21 backport is in progress.

[shruacha1234]

/approval 8294399 request the backport fix. The FileDispatcherImpl class includes native methods close0(), dup0(), and preClose0(), which are commented as:
Shared with SocketDispatcher and DatagramDispatcher but NOT used by FileDispatcherImpl
These methods are being relocated to a new class, UnixDispatcher, which is referenced by both SocketDispatcher and DatagramDispatcher, where the methods are actually used.
Additionally, the FileDispatcherImpl class contains preClose() and dup() methods that are not invoked anywhere and can therefore be safely removed.

I’m backporting 8294399 because it introduces the UnixDispatcher class, which is a necessary foundation for other functional fixes, including the one in openjdk/jdk#25817. Although it’s a refactoring change, it helps separate preClose() and dup() method logic into a dedicated class, making the code cleaner and easier to maintain. More importantly, the fix that address real issues (async close handling and race conditions) are built on top of this refactoring. Without backporting 8294399, those fixes cannot be cleanly or safely applied to JDK 17.

I was able to create a successful AIX local build with this patch. Also, I ran the java/net and java/nio jtreg tests, and all are passing

[openjdk]

@shruacha1234
8294399: Approval has already been requested and rejected.

[bridgekeeper]

@shruacha1234 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@shruacha1234 This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[shruacha1234]

/open

[openjdk]

@shruacha1234 This pull request is now open

[shruacha1234]

GoeLin

Also, if you want to backport https://bugs.openjdk.org/browse/JDK-8317801, please first backport it to 25, then 21. Please read https://wiki.openjdk.org/display/JDKUpdates/How+to+contribute+or+backport+a+fix , it e.g. advises you to first backport to higher releases. Thanks.

https://bugs.openjdk.org/browse/JDK-8317801 fix is now part of both JDK25 and JDK21. Can you please review this PR with the below justification

/approval 8294399 request the backport fix. The FileDispatcherImpl class includes native methods close0(), dup0(), and preClose0(), which are commented as: Shared with SocketDispatcher and DatagramDispatcher but NOT used by FileDispatcherImpl These methods are being relocated to a new class, UnixDispatcher, which is referenced by both SocketDispatcher and DatagramDispatcher, where the methods are actually used. Additionally, the FileDispatcherImpl class contains preClose() and dup() methods that are not invoked anywhere and can therefore be safely removed.

I’m backporting 8294399 because it introduces the UnixDispatcher class, which is a necessary foundation for other functional fixes, including the one in openjdk/jdk#25817. Although it’s a refactoring change, it helps separate preClose() and dup() method logic into a dedicated class, making the code cleaner and easier to maintain. More importantly, the fix that address real issues (async close handling and race conditions) are built on top of this refactoring. Without backporting 8294399, those fixes cannot be cleanly or safely applied to JDK 17.

I was able to create a successful AIX local build with this patch. Also, I ran the java/net and java/nio jtreg tests, and all are passing

so that I can backport https://bugs.openjdk.org/browse/JDK-8317801 to JDK17 as well