8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner #782

lahodaj · 2020-10-21T13:12:03Z

…orStateException: current thread is not owner

The KeepAliveStreamCleaner is calling Condition.wait (i.e. the Object.wait variant), should presumably be calling Condition.await. (NetBeans is producing warnings on the line as well.)

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Testing

	Linux x32	Linux x64	Windows x64	macOS x64
Build	✔️ (1/1 passed)	✔️ (5/5 passed)	✔️ (2/2 passed)	✔️ (2/2 passed)
Test (tier1)		✔️ (9/9 passed)	❌ (1/9 failed)	✔️ (9/9 passed)

Failed test task

Windows x64 (hs/tier1 compiler)

Issue

JDK-8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner

Reviewers

Alan Bateman (@AlanBateman - Reviewer)
Chris Hegarty (@ChrisHegarty - Reviewer)
Daniel Fuchs (@dfuch - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/782/head:pull/782
$ git checkout pull/782

…orStateException: current thread is not owner

bridgekeeper · 2020-10-21T13:12:43Z

👋 Welcome back jlahoda! 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 · 2020-10-21T13:15:14Z

@lahodaj The following label will be automatically applied to this pull request:

net

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.

mlbridge · 2020-10-21T13:20:24Z

Webrevs

01: Full - Incremental (143aa18)
00: Full (17f6956)

AlanBateman · 2020-10-21T13:41:15Z

test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java

@@ -133,6 +139,9 @@ public static void clientHttpCalls() {
        } finally {
            server.terminate();
        }
+        if (!uncaught.isEmpty()) {
+            throw new AssertionError("Unhandled exception:", uncaught.get(0));


The existing test throws RuntimeException when it fails, maybe this case should do the same.

…, as suggested by alanb

openjdk · 2020-10-22T07:27:12Z

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

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

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

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner

Reviewed-by: alanb, chegar, dfuchs

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 20 new commits pushed to the master branch:

299e115: 8198395: Test javax/swing/plaf/metal/MetalLookAndFeel/Test8039750.java fails in mach5
a5b7bc5: 7156347: javax/swing/JList/6462008/bug6462008.java fails
b25d894: 8252204: AArch64: Implement SHA3 accelerator/intrinsic
7d3d4da: 8240709: Enable javax/swing/UI/UnninstallUIMemoryLeaks/UnninstallUIMemoryLeaks.java on all L&F
5d26229: 8255174: Vector API unit tests for missed public api code coverage
b9186be: 6606767: resexhausted00[34] fail assert(!thread->owns_locks(), "must release all locks when leaving VM")
1191a63: 8199697: FIPS 186-4 RSA Key Generation
60d3fa2: 8255022: Documentation missing for Vector API zero methods
9ade94b: 8206311: Add docs-javase, docs-reference to CI build
3445031: 8255200: ProblemList com/sun/jdi/EATests.java for ZGC
... and 10 more: https://git.openjdk.java.net/jdk/compare/e559bd2c8bf64888c8890605839d909228c97aaa...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

lahodaj · 2020-10-22T07:28:58Z

/integrate

openjdk · 2020-10-22T07:32:47Z

@lahodaj Since your change was applied there have been 20 commits pushed to the master branch:

299e115: 8198395: Test javax/swing/plaf/metal/MetalLookAndFeel/Test8039750.java fails in mach5
a5b7bc5: 7156347: javax/swing/JList/6462008/bug6462008.java fails
b25d894: 8252204: AArch64: Implement SHA3 accelerator/intrinsic
7d3d4da: 8240709: Enable javax/swing/UI/UnninstallUIMemoryLeaks/UnninstallUIMemoryLeaks.java on all L&F
5d26229: 8255174: Vector API unit tests for missed public api code coverage
b9186be: 6606767: resexhausted00[34] fail assert(!thread->owns_locks(), "must release all locks when leaving VM")
1191a63: 8199697: FIPS 186-4 RSA Key Generation
60d3fa2: 8255022: Documentation missing for Vector API zero methods
9ade94b: 8206311: Add docs-javase, docs-reference to CI build
3445031: 8255200: ProblemList com/sun/jdi/EATests.java for ZGC
... and 10 more: https://git.openjdk.java.net/jdk/compare/e559bd2c8bf64888c8890605839d909228c97aaa...master

Your commit was automatically rebased without conflicts.

Pushed as commit 211bb62.

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

lahodaj added 2 commits October 21, 2020 14:45

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonit…

cbf4fe9

…orStateException: current thread is not owner

Adding bug number to the test.

17f6956

openjdk bot added the rfr Pull request is ready for review label Oct 21, 2020

openjdk bot added the net net-dev@openjdk.org label Oct 21, 2020

AlanBateman reviewed Oct 21, 2020

View reviewed changes

AlanBateman approved these changes Oct 21, 2020

View reviewed changes

When the test fails, throw RuntimeException instead of AssertionError…

143aa18

…, as suggested by alanb

AlanBateman approved these changes Oct 21, 2020

View reviewed changes

ChrisHegarty approved these changes Oct 21, 2020

View reviewed changes

dfuch approved these changes Oct 21, 2020

View reviewed changes

lahodaj changed the title ~~8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonit…~~ 8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner Oct 22, 2020

openjdk bot added the ready Pull request is ready to be integrated label Oct 22, 2020

openjdk bot closed this Oct 22, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 22, 2020

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner #782

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner #782

lahodaj commented Oct 21, 2020 •

edited by openjdk bot

bridgekeeper bot commented Oct 21, 2020

openjdk bot commented Oct 21, 2020

mlbridge bot commented Oct 21, 2020 •

edited

AlanBateman Oct 21, 2020

openjdk bot commented Oct 22, 2020

lahodaj commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner #782

8255124: KeepAliveStreamCleaner may crash with java.lang.IllegalMonitorStateException: current thread is not owner #782

Conversation

lahodaj commented Oct 21, 2020 • edited by openjdk bot

Progress

Testing

Issue

Reviewers

Download

bridgekeeper bot commented Oct 21, 2020

openjdk bot commented Oct 21, 2020

mlbridge bot commented Oct 21, 2020 • edited

Webrevs

AlanBateman Oct 21, 2020

Choose a reason for hiding this comment

openjdk bot commented Oct 22, 2020

lahodaj commented Oct 22, 2020

openjdk bot commented Oct 22, 2020

lahodaj commented Oct 21, 2020 •

edited by openjdk bot

mlbridge bot commented Oct 21, 2020 •

edited