8317801: java/net/Socket/asyncClose/Race.java fails intermittently (aix) #21088
Conversation
|
👋 Welcome back varadam! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@varada1110 The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
|
Hi @TheRealMDoerr, could you please review the code. Same workaround as #21087. Here the test doesn't seem to pick up the code changes from the file created under aix. I would like to know which is better patch ? |
|
/contributor add Shruthi.Shruthi1 [email protected] |
|
@varada1110 |
|
The integration blockers above must get fixed: "Change must not contain extraneous whitespace", "Commit message must refer to an issue" |
8317801: java/net/Socket/asyncClose/Race.java fails intermittently (aix) Co-authored-by: Shruthi.Shruthi1 <[email protected]> Signed-off-by: Varada M <[email protected]>
varada1110
changed the title
varada1110
force-pushed
the
PRECLOSE
branch
from
54155e2
to
a4961b2
Compare
|
I have fixed whitespace errors and commit message. Also I have added explanation in the description |
|
I think that using an extra file only for AIX is not supported by the build. It's not picked up. Anyway, this PR is a workaround for an AIX problem. We prefer waiting for the AIX fix instead of modifying the JDK. |
| @@ -0,0 +1,236 @@ | |||
| /* | |||
| * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. | |||
There was a problem hiding this comment.
| * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. | |
| * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. | |
| * Copyright 2024 IBM Corporation. All rights reserved. |
| private static final NativeDispatcher nd = new SocketDispatcher(); | ||
| private final FileDescriptor fd = new FileDescriptor(); | ||
|
|
||
| // private final FileDescriptor fd; |
There was a problem hiding this comment.
i see few comments with funny alignment, maybe fix them as well before making it rfr :-)
The preClose() method internally calls the dup2() system call. If there is a dup2() call on a file descriptor on a thread while another thread is blocked in a read/write operation on the same file descriptor, then the dup2() call is known to hang. Currently, preClose() experiences a hang because we call dup2() before killing the reader/writer thread(s). The workaround is to first kill the reader/writer thread(s) and then do the preClose() sequence. This reordering in the different channel implementations resolves the problem.
This is an AIX specific patch by bringing the code change under AIX folder. However, we noticed that the test is not picking up the new code changes. It is going back to old code NioSocketImpl.java#L876.
Therefore, we used System.property and checked for AIX platform pull/21087
We would like to know which is better patch and why the new code change is not recognized by the test.
JBS Issue : JDK-8317801
Progress
Issue
Contributors
<[email protected]>Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21088/head:pull/21088$ git checkout pull/21088Update a local copy of the PR:
$ git checkout pull/21088$ git pull https://git.openjdk.org/jdk.git pull/21088/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21088View PR using the GUI difftool:
$ git pr show -t 21088Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21088.diff