Re: [8u] RFR: 8225690: Multiple AttachListener threads can be created

Yi Yang qingfeng.yy at alibaba-inc.com
Thu Oct 21 08:08:16 UTC 2021


Hi Paul,

Currently, Atomic::load only accepts jlong as a parameter, so I use some macros to define AL_XX values and change type of _state to `volatile jlong`, what do you think?

Introduction LingeredApp has a relatively large effort and may also be relatively risky. There are already similar examples of removing @test tag when using LingeredApp(ClhsdbJstackXcompStress, LingeredAppWithRecComputation), so this may be a feasible solution. These tests are all passed(w/ some modifications to allow it working on JDK8u). Nevertheless, I could investigate if it's possible to backport LingeredApp to  after these patches.

I have prepared all related changeset. Please help review it, thank you.

-yyang

1. 8225690: Multiple AttachListener threads can be created
Original Issue: https://bugs.openjdk.java.net/browse/JDK-8225690
Original Changeset: https://hg.openjdk.java.net/jdk/jdk/rev/70fab3a8ff02
Backport Issue: https://bugs.openjdk.java.net/browse/JDK-8274953
Backport Changeset: http://cr.openjdk.java.net/~yyang/8274953/webrev.00/
Review Effort: M

2. 8227815: Minimal VM: set_state is not a member of AttachListener
Original Issue: https://bugs.openjdk.java.net/browse/JDK-8227815
Original Changeset: https://hg.openjdk.java.net/jdk/jdk/rev/2660d47140da
Backport Issue: https://bugs.openjdk.java.net/browse/JDK-8275694
Backport Changeset: https://cr.openjdk.java.net/~yyang/8275694/webrev/
Review Effort: XS

3. 8227738: jvmti/DataDumpRequest/datadumpreq001 failed due to "exit code is 134"
Original Issue: https://bugs.openjdk.java.net/browse/JDK-8227738
Original Changeset: https://hg.openjdk.java.net/jdk/jdk/rev/4888ccfc234e
Backport Issue: https://bugs.openjdk.java.net/browse/JDK-8275695
Backport Changeset: https://cr.openjdk.java.net/~yyang/8275695/webrev/Review Effort: XS


------------------------------------------------------------------
From:Hohensee, Paul <hohensee at amazon.com>
Send Time:2021 Oct. 16 (Sat.) 04:39
To:"YANG, Yi" <qingfeng.yy at alibaba-inc.com>; jdk8u-dev at openjdk.java.net <jdk8u-dev at openjdk.java.net>
Subject:Re: [8u] RFR: 8225690: Multiple AttachListener threads can be created

In attachListener.hpp, the AttachListenerState enum's underlying type will be int, so casting to jlong for the atomic operations will clobber whatever is beyond _state, and won't work at all on big-endian machines. C++11 adds enum base type specification, but we can't use C++11 features in 8u. I would define const int AL_* values and declare _state to be an int.

I'm leery of disabling the test. Please see if you can backport 8165500, which introduced LingeredApp.

You have code from 8237499 in the patch which has already been backported. Please remove it. Also, please post review requests for 8227815 and 8227738 backports, with the intent to push all three at once.

Thanks,
Paul

 -----Original Message-----
From: jdk8u-dev <jdk8u-dev-retn at openjdk.java.net> on behalf of Yi Yang <qingfeng.yy at alibaba-inc.com>
Reply-To: Yi Yang <qingfeng.yy at alibaba-inc.com>
Date: Friday, October 8, 2021 at 4:21 AM
To: "jdk8u-dev at openjdk.java.net" <jdk8u-dev at openjdk.java.net>
Subject: [8u] RFR: 8225690: Multiple AttachListener threads can be created

Hi,

Can I have a review for this backport of JDK-8225690?
Original Issue: https://bugs.openjdk.java.net/browse/JDK-8225690
Original Changeset: https://hg.openjdk.java.net/jdk/jdk/rev/70fab3a8ff02
Backport Issue: https://bugs.openjdk.java.net/browse/JDK-8274953
Backport Changeset: http://cr.openjdk.java.net/~yyang/8274953/

VM can't be attached If someone cleans up the /tmp directory. This patch
addresses the problem. It can not apply cleanly, some minor conflicts are as follows:

1. Atomic::store(new_state, &_state)
jdk8u does not have templated Atomic function family, use explicitly type
conversion `Atomic::store((jlong)new_state, (volatile jlong*)&_state)`.

2. IsRegisteredEnum<AttachListenerState>
jdk8u does not have metaprogramming type traits,
`template<> struct IsRegisteredEnum<AttachListenerState> : public TrueType {};`
simply omit it.

3. os::naked_yield
os::naked_yield is missing due to https://bugs.openjdk.java.net/browse/JDK-8047714,
use os::yield instead.

4. log_debug(attach)("...")
jdk8u does not have unified logging system. Use the following code:
`debug_only(warning("..."))`

5. Test
They can not work because of the usages of LingeredApp. Remove @test tag to
disable them while keeping their source code.

Test cases have been passed manually. The original issue has related bugs:
https://bugs.openjdk.java.net/browse/JDK-8227815
https://bugs.openjdk.java.net/browse/JDK-8235211
https://bugs.openjdk.java.net/browse/JDK-8227738
I'd like to create follow-up backports(JDK-8227815 and JDK-8227738) once this
merged. JDK-8235211 is not necessary since RemovingUnixDomainSocketTest.java
will be ignored as I said before.

Thanks!



More information about the jdk8u-dev mailing list