8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file #32

navyxliu · 2022-04-05T00:55:22Z

Hi,

Could you review this backport of 8235211? This is a follow up of JDK-8225690.

This patch can't be applied to jdk8u cleanly. We replace os::naked_yield with os::yield(), which is available in jdk8u and have same implementations on Linux/BSD/AIX. Except for that, all other changes are just path adjustment.

There's a deadlock in the original backport. Both Signal dispatcher(refer to S for short) and AttachListener(A) are Java Threads. Therefore, they are subject to safepoint synchronization.

A sets _state to AL_INITIALIZED and then get blocked at a safepoint(maybe AttachListener::dequeue).
If the socket file is deleted and JVM receive SIGQUIT the, S realizes that the socket file has gone in check_socket_file and start to reinitialize it. It falls into this loop and prevent itself from reaching the safepoint.

     while (AttachListener::transit_state(AL_INITIALIZING,
                                           AL_NOT_INITIALIZED) != AL_NOT_INITIALIZED) {
        os::naked_yield();
      }

If A waked up and realized that the socket has gone, it would set _state to AL_NOT_INITIALIZED. Current code is adeadlock. A can’t wake up because S hasn’t reached the safepoint yet. S can’t reach the safepoint because A hasn’t set _state to AL_NOT_INITIALIZED. This patch avoids the deadlock by trapping into blocked state before the loop.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 reviewer)

Issues

JDK-8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file
JDK-8244973: serviceability/attach/RemovingUnixDomainSocketTest.java fails "stderr was not empty"

Reviewers

Paul Hohensee (@phohensee - Reviewer)
Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk8u-dev pull/32/head:pull/32
$ git checkout pull/32

Update a local copy of the PR:
$ git checkout pull/32
$ git pull https://git.openjdk.java.net/jdk8u-dev pull/32/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 32

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk8u-dev/pull/32.diff

…s with AttachNotSupportedException: Unable to open socket file

…s "stderr was not empty"

bridgekeeper · 2022-04-05T00:55:30Z

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

mlbridge · 2022-04-05T01:14:11Z

Webrevs

openjdk · 2022-04-05T01:31:05Z

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

navyxliu · 2022-04-05T01:52:35Z

/issue add JDK-8244973

openjdk · 2022-04-05T01:53:05Z

@navyxliu
Adding additional issue to issue list: 8244973: serviceability/attach/RemovingUnixDomainSocketTest.java fails "stderr was not empty".

phohensee

I compared with openjdk/jdk11u-dev#153.

Lgtm, except there's a missing import in RemovingUnixDomainSocketTest.java and LingeredApp.java, vis

import jdk.test.lib.Utils;

@test

Currently, RemovingUnixDomainSocketTest.java isn't running because @test is taken away when we backport JDK-8225690. we try to keep the patch intact in case we enable it in the future.

phohensee

Lgtm.

openjdk · 2022-04-06T15:35:12Z

@navyxliu This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file
8244973: serviceability/attach/RemovingUnixDomainSocketTest.java fails "stderr was not empty"

Reviewed-by: phh, andrew

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 35 new commits pushed to the master branch:

1bc3be2: Merge
ee82a7d: 8282458: Update .jcheck/conf file for 8u move to git
d91ee59: Added tag jdk8u332-ga for changeset 37aca7715d13
607b14e: 8285445: cannot open file "NUL:"
62defc3: 8284772: 8u GHA: Use GCC Major Version Dependencies Only
65c4a5d: Merge
cc541e9: Added tag jdk8u332-b09 for changeset 37aca7715d13
3d2fe9b: 8284936: Fix Java 7 bootstrap breakage due to use of Arrays.stream
c7a735d: Added tag jdk8u332-b08 for changeset 95b31159fdfd
d0b8929: 8284920: Incorrect Token type causes XPath expression to return empty result
... and 25 more: https://git.openjdk.java.net/jdk8u-dev/compare/c5ca29fd4a582fd390595c1c50771dc02ba0f9ba...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@phohensee, @gnu-andrew) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

bridgekeeper · 2022-05-04T18:05:08Z

@navyxliu 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 add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

gnu-andrew

Looks good to me. Clean backport bar the naked_yield/yield change.

gnu-andrew · 2022-05-09T03:00:23Z

Added jdk8u-fix-yes.

navyxliu · 2022-05-09T15:55:25Z

/integrate

openjdk · 2022-05-09T15:56:56Z

@navyxliu
Your change (at version 6f86e09) is now ready to be sponsored by a Committer.

phohensee · 2022-05-09T16:37:13Z

/sponsor

openjdk · 2022-05-09T16:38:21Z

Going to push as commit 1067c54.
Since your change was applied there have been 35 commits pushed to the master branch:

1bc3be2: Merge
ee82a7d: 8282458: Update .jcheck/conf file for 8u move to git
d91ee59: Added tag jdk8u332-ga for changeset 37aca7715d13
607b14e: 8285445: cannot open file "NUL:"
62defc3: 8284772: 8u GHA: Use GCC Major Version Dependencies Only
65c4a5d: Merge
cc541e9: Added tag jdk8u332-b09 for changeset 37aca7715d13
3d2fe9b: 8284936: Fix Java 7 bootstrap breakage due to use of Arrays.stream
c7a735d: Added tag jdk8u332-b08 for changeset 95b31159fdfd
d0b8929: 8284920: Incorrect Token type causes XPath expression to return empty result
... and 25 more: https://git.openjdk.java.net/jdk8u-dev/compare/c5ca29fd4a582fd390595c1c50771dc02ba0f9ba...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-05-09T16:38:38Z

@phohensee @navyxliu Pushed as commit 1067c54.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Xin Liu added 2 commits April 5, 2022 00:14

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fail…

0e5a55d

…s with AttachNotSupportedException: Unable to open socket file

8244973: serviceability/attach/RemovingUnixDomainSocketTest.java fail…

84d4217

…s "stderr was not empty"

openjdk bot added the rfr Pull request is ready for review label Apr 5, 2022

navyxliu changed the title ~~8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file~~ Backport 073e095e6053550b17b1daf33df2be4f4c4b40ad Apr 5, 2022

openjdk bot changed the title ~~Backport 073e095e6053550b17b1daf33df2be4f4c4b40ad~~ 8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file Apr 5, 2022

openjdk bot added the backport label Apr 5, 2022

adjust alignment. use original patch's style.

c9df082

phohensee suggested changes Apr 5, 2022

View reviewed changes

fix typo when we manually edited the two test files.

6f86e09

Currently, RemovingUnixDomainSocketTest.java isn't running because @test is taken away when we backport JDK-8225690. we try to keep the patch intact in case we enable it in the future.

phohensee approved these changes Apr 6, 2022

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 6, 2022

gnu-andrew approved these changes May 9, 2022

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label May 9, 2022

openjdk bot added the integrated Pull request has been integrated label May 9, 2022

openjdk bot closed this May 9, 2022

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels May 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file #32

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file #32

navyxliu commented Apr 5, 2022 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 5, 2022

mlbridge bot commented Apr 5, 2022 •

edited

Loading

openjdk bot commented Apr 5, 2022

navyxliu commented Apr 5, 2022

openjdk bot commented Apr 5, 2022

phohensee left a comment

phohensee left a comment

openjdk bot commented Apr 6, 2022 •

edited

Loading

bridgekeeper bot commented May 4, 2022

gnu-andrew left a comment

gnu-andrew commented May 9, 2022

navyxliu commented May 9, 2022

openjdk bot commented May 9, 2022

phohensee commented May 9, 2022

openjdk bot commented May 9, 2022

openjdk bot commented May 9, 2022

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file #32

8235211: serviceability/attach/RemovingUnixDomainSocketTest.java fails with AttachNotSupportedException: Unable to open socket file #32

Conversation

navyxliu commented Apr 5, 2022 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Apr 5, 2022

mlbridge bot commented Apr 5, 2022 • edited Loading

Webrevs

openjdk bot commented Apr 5, 2022

navyxliu commented Apr 5, 2022

openjdk bot commented Apr 5, 2022

phohensee left a comment

Choose a reason for hiding this comment

phohensee left a comment

Choose a reason for hiding this comment

openjdk bot commented Apr 6, 2022 • edited Loading

bridgekeeper bot commented May 4, 2022

gnu-andrew left a comment

Choose a reason for hiding this comment

gnu-andrew commented May 9, 2022

navyxliu commented May 9, 2022

openjdk bot commented May 9, 2022

phohensee commented May 9, 2022

openjdk bot commented May 9, 2022

openjdk bot commented May 9, 2022

navyxliu commented Apr 5, 2022 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 5, 2022 •

edited

Loading

openjdk bot commented Apr 6, 2022 •

edited

Loading