Integrated: 8259627: Potential memory leaks in JVMTI after JDK-8227745
On Tue, 12 Jan 2021 19:09:43 GMT, Richard Reingruber wrote: > This change eliminates memory leaks in the JVMTI implementation reported by > SonarCloud. > > The leaks occur when the Java heap is exhausted. This pull request has now been integrated. Changeset: 6d4a593f Author:Richard Reingruber URL: https://git.openjdk.java.net/jdk/commit/6d4a593f Stats: 16 lines in 1 file changed: 8 ins; 8 del; 0 mod 8259627: Potential memory leaks in JVMTI after JDK-8227745 Reviewed-by: shade, stuefe, dholmes, sspitsyn - PR: https://git.openjdk.java.net/jdk/pull/2055
RFR: 8065773: JDI: UOE is not thrown, when redefineClasses changes a class modifier
The test failed because it expects that public/protected/default/private and static modifiers differ on the JVM level like in Java source code. However, only the ACC_PUBLIC modifier has an effect on interfaces. Here is my proposal from bug comments: I looked at the test and checked bytecode and spec. Indeed, the bytecode of all redefineclasses021bi redefined classes differs only by ACC_PUBLIC attribute. So there is no sense to test other access levels even they exist in JLS. The last redefinition adds 'static' modifier and verifies that there is no UOE is thrown. However static modifiers are also not set for interfaces because according to JLS it is set implicitly. https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html#jls-8.5.1 "A member interface is implicitly static (§9.1.1). It is permitted for the declaration of a member interface to redundantly specify the static modifier." The test already has been fixed to verify that UOE is not thrown but it just doesn't do anything, assuming that bytecode is the same. So I believe this test case might safely be deleted. It is also InnerClasses_attribute in redefineclasses021b which points to attributes of the inner class. However, the spec says that it used by the compiler only. Also, the test doesn't redefine this class but interface only. See https://docs.oracle.com/javase/specs/jvms/se13/html/jvms-4.html: "inner_class_access_flags The value of the inner_class_access_flags item is a mask of flags used to denote access permissions to and properties of the class or interface C as declared in the source code from which this class file was compiled. It is used by a compiler to recover the original information when the source code is not available. The flags are specified in Table 4.7.6-A." So I think it is enough just to check public vs not public access modifiers. - Commit messages: - 8065773: JDI: UOE is not thrown, when redefineClasses changes a class modifier Changes: https://git.openjdk.java.net/jdk/pull/2093/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2093=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8065773 Stats: 702 lines in 13 files changed: 31 ins; 636 del; 35 mod Patch: https://git.openjdk.java.net/jdk/pull/2093.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2093/head:pull/2093 PR: https://git.openjdk.java.net/jdk/pull/2093
Integrated: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
On Sat, 5 Sep 2020 14:26:17 GMT, Yasumasa Suenaga wrote:
> If `Agent_OnAttach()` in JVMTI agent which is attempted to load via
> JVMTI.agent_load dcmd is failed, it would not be unloaded.
> We've [discussed it on
> serviceability-dev](https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-September/032839.html).
> This PR is a continuation of that.
>
> This PR also includes to call `Agent_OnUnload()` when `Agent_OnAttach()`
> failed.
>
> How to reproduce:
>
> 1. Build JVMTI agent for test
> $ git clone https://github.com/YaSuenag/jvmti-examples.git
> $ cd jvmti-examples/helloworld/out/build
> $ cmake ../..
>
> 2. Run JShell
>
> 3. Load JVMTI agent via `jcmd JVMTI.agent_load` with "error" ("error"
> means `Agent_OnAttach()` returns JNI_ERR)
> $ jcmd
> 89456 jdk.jshell.execution.RemoteExecutionControl 45651
> 89547 sun.tools.jcmd.JCmd
> 89436 jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider
> $ jcmd 89436 JVMTI.agent_load `pwd`/libhelloworld.so error
> 89436:
> return code: -1
>
> 4. Check loaded libraries via `jcmd VM.dynlibs`
> $ jcmd 89436 VM.dynlibs | grep libhelloworld
> 7f2f8b06b000-7f2f8b06c000 r--p fd:00 11818202
> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
> 7f2f8b06c000-7f2f8b06d000 r-xp 1000 fd:00 11818202
> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
> 7f2f8b06d000-7f2f8b06e000 r--p 2000 fd:00 11818202
> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
> 7f2f8b06e000-7f2f8b06f000 r--p 2000 fd:00 11818202
> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
> 7f2f8b06f000-7f2f8b07 rw-p 3000 fd:00 11818202
> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
This pull request has now been integrated.
Changeset: 90960c5f
Author:Yasumasa Suenaga
URL: https://git.openjdk.java.net/jdk/commit/90960c5f
Stats: 15 lines in 3 files changed: 10 ins; 0 del; 5 mod
8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed
Reviewed-by: dholmes, sspitsyn
-
PR: https://git.openjdk.java.net/jdk/pull/19
Re: RFR: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed [v3]
On Thu, 14 Jan 2021 03:08:19 GMT, Yasumasa Suenaga wrote:
>> If `Agent_OnAttach()` in JVMTI agent which is attempted to load via
>> JVMTI.agent_load dcmd is failed, it would not be unloaded.
>> We've [discussed it on
>> serviceability-dev](https://mail.openjdk.java.net/pipermail/serviceability-dev/2020-September/032839.html).
>> This PR is a continuation of that.
>>
>> This PR also includes to call `Agent_OnUnload()` when `Agent_OnAttach()`
>> failed.
>>
>> How to reproduce:
>>
>> 1. Build JVMTI agent for test
>> $ git clone https://github.com/YaSuenag/jvmti-examples.git
>> $ cd jvmti-examples/helloworld/out/build
>> $ cmake ../..
>>
>> 2. Run JShell
>>
>> 3. Load JVMTI agent via `jcmd JVMTI.agent_load` with "error" ("error"
>> means `Agent_OnAttach()` returns JNI_ERR)
>> $ jcmd
>> 89456 jdk.jshell.execution.RemoteExecutionControl 45651
>> 89547 sun.tools.jcmd.JCmd
>> 89436 jdk.jshell/jdk.internal.jshell.tool.JShellToolProvider
>> $ jcmd 89436 JVMTI.agent_load `pwd`/libhelloworld.so error
>> 89436:
>> return code: -1
>>
>> 4. Check loaded libraries via `jcmd VM.dynlibs`
>> $ jcmd 89436 VM.dynlibs | grep libhelloworld
>> 7f2f8b06b000-7f2f8b06c000 r--p fd:00 11818202
>> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
>> 7f2f8b06c000-7f2f8b06d000 r-xp 1000 fd:00 11818202
>> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
>> 7f2f8b06d000-7f2f8b06e000 r--p 2000 fd:00 11818202
>> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
>> 7f2f8b06e000-7f2f8b06f000 r--p 2000 fd:00 11818202
>> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
>> 7f2f8b06f000-7f2f8b07 rw-p 3000 fd:00 11818202
>> /home/ysuenaga/github/jvmti-examples/helloworld/out/build/libhelloworld.so
>
> Yasumasa Suenaga has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains six commits:
>
> - Update jvmti.xml to follow CSR
> - Merge remote-tracking branch 'upstream/master' into JDK-8252657
> - Update patch
> - Merge remote-tracking branch 'upstream/master' into JDK-8252657
> - revert
> - JVMTI agent is not unloaded when Agent_OnAttach is failed
Hi Yasumasa,
Both the CSR and the fix look good.
Thank you for your patience with the process.
Thanks,
Serguei
-
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/19
Integrated: 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping
On Fri, 18 Dec 2020 02:01:28 GMT, Chris Plummer wrote: > There is an assert in `JvmtiThreadState::cur_stack_depth()` that tends to > slow down single stepping a lot when running the debuggee with a debug jvm. > See CR for details. The fix is to allow disabling of this assert using the > new EnableJVMTIStackDepthAsserts global, which defaults to true. This pull request has now been integrated. Changeset: 4f881ba5 Author:Chris Plummer URL: https://git.openjdk.java.net/jdk/commit/4f881ba5 Stats: 12 lines in 2 files changed: 8 ins; 0 del; 4 mod 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping Reviewed-by: sspitsyn, dholmes, amenkov - PR: https://git.openjdk.java.net/jdk/pull/1835
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect [v2]
On Thu, 14 Jan 2021 20:32:17 GMT, Leonid Mesnik wrote:
>> est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of
>> strcmp results here:
>>
>> for (i=0; i> if (strcmp(methNam,METHODS[i][0]) &&
>> strcmp(methSig,METHODS[i][1])) {
>> printf("CHECK PASSED: method name: "%s"\tsignature: "%s" %d\n",
>>methNam, methSig, i);
>> if (checkStatus == PASSED)
>> bpEvents[i]++;
>> break;
>> }
>>
>> So test passed when both strcmp (name,sig) are not zero.
>>
>> The test passes only because there are 2 methods that are checked and it
>> increases counters for "incorrect" methods.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> copyrights and ident fixed
Marked as reviewed by cjplummer (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/2084
Re: RFR: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed [v3]
On 14/01/2021 6:29 pm, Yasumasa Suenaga wrote: On Thu, 14 Jan 2021 07:19:06 GMT, David Holmes wrote: Hi Yasumasa, Spec update looks good. VM tweak to call dll_unload on failure looks good. Thanks! I don't understand the test change though. To check the agent is unloaded, call VM.dynlibs dcmd and check whether agent path is not contained. libReturnError.so would be loaded only once in AttachReturnError.java, so it is expected to unload when `Agent_OnAttach()` fails. Got it! Thanks. David - - PR: https://git.openjdk.java.net/jdk/pull/19
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect [v2]
On Thu, 14 Jan 2021 20:32:17 GMT, Leonid Mesnik wrote:
>> est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of
>> strcmp results here:
>>
>> for (i=0; i> if (strcmp(methNam,METHODS[i][0]) &&
>> strcmp(methSig,METHODS[i][1])) {
>> printf("CHECK PASSED: method name: "%s"\tsignature: "%s" %d\n",
>>methNam, methSig, i);
>> if (checkStatus == PASSED)
>> bpEvents[i]++;
>> break;
>> }
>>
>> So test passed when both strcmp (name,sig) are not zero.
>>
>> The test passes only because there are 2 methods that are checked and it
>> increases counters for "incorrect" methods.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> copyrights and ident fixed
Hi Leonid,
LGTM++
Thanks,
Serguei
-
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2084
Integrated: 8259350: Add some internal debugging APIs to the debug agent
On Thu, 7 Jan 2021 04:28:19 GMT, Chris Plummer wrote: > This PR adds some APIs that are useful when debugging the debug agent. They > can be called from gdb or from other parts of the debug agent. They mostly do > things like dumping internal data structures that are tedious to dump or > iterate over in gdb. I developed them while working on loom and found them > useful. > > Note that `dumpThreadList()` and `dumpThread()` are not exported from > threadControl.c because the argument types are only visible within > threadControl.c. > > I debated whether to bracket all these APIs with `#ifdef DEBUG`. In the end I > decided to in order to make it clear they are only meant for debugging > purposes. If you temporarily need them with a product build, you can always > modify the code to include them. I could be persuaded `#ifdef DEBUG` though. This pull request has now been integrated. Changeset: d18d26e8 Author:Chris Plummer URL: https://git.openjdk.java.net/jdk/commit/d18d26e8 Stats: 258 lines in 8 files changed: 250 ins; 0 del; 8 mod 8259350: Add some internal debugging APIs to the debug agent Reviewed-by: sspitsyn, amenkov - PR: https://git.openjdk.java.net/jdk/pull/1970
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect [v2]
On Thu, 14 Jan 2021 20:32:17 GMT, Leonid Mesnik wrote:
>> est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of
>> strcmp results here:
>>
>> for (i=0; i> if (strcmp(methNam,METHODS[i][0]) &&
>> strcmp(methSig,METHODS[i][1])) {
>> printf("CHECK PASSED: method name: "%s"\tsignature: "%s" %d\n",
>>methNam, methSig, i);
>> if (checkStatus == PASSED)
>> bpEvents[i]++;
>> break;
>> }
>>
>> So test passed when both strcmp (name,sig) are not zero.
>>
>> The test passes only because there are 2 methods that are checked and it
>> increases counters for "incorrect" methods.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> copyrights and ident fixed
Marked as reviewed by iignatyev (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/2084
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect [v2]
On Thu, 14 Jan 2021 19:22:07 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> copyrights and ident fixed > > test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp > line 173: > >> 171: >> 172: for (i=0; i> 173: if (strcmp(methNam,METHODS[i][0]) == 0 && > > could you please add space before `,`? done > test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp > line 2: > >> 1: /* >> 2: * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights >> reserved. > > it's 2021 ough, thanks! - PR: https://git.openjdk.java.net/jdk/pull/2084
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect [v2]
> est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of
> strcmp results here:
>
> for (i=0; i if (strcmp(methNam,METHODS[i][0]) &&
> strcmp(methSig,METHODS[i][1])) {
> printf("CHECK PASSED: method name: "%s"\tsignature: "%s" %d\n",
>methNam, methSig, i);
> if (checkStatus == PASSED)
> bpEvents[i]++;
> break;
> }
>
> So test passed when both strcmp (name,sig) are not zero.
>
> The test passes only because there are 2 methods that are checked and it
> increases counters for "incorrect" methods.
Leonid Mesnik has updated the pull request incrementally with one additional
commit since the last revision:
copyrights and ident fixed
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/2084/files
- new: https://git.openjdk.java.net/jdk/pull/2084/files/009db2ef..c7b78f85
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=2084=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=2084=00-01
Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
Patch: https://git.openjdk.java.net/jdk/pull/2084.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2084/head:pull/2084
PR: https://git.openjdk.java.net/jdk/pull/2084
Fwd: CSR: 8259798: Add jcmd option to dump CDS
Oops, push send too soon. Thanks Yumin Forwarded Message Subject:CSR: 8259798: Add jcmd option to dump CDS Date: Thu, 14 Jan 2021 12:03:38 -0800 From: Yumin Qi To: hotspot-runtime-...@openjdk.java.net Hi, All I have created bug and CSR for this issue and wish have your comments on it: BUG: https://bugs.openjdk.java.net/browse/JDK-8259070 CSR: https://bugs.openjdk.java.net/browse/JDK-8259798 The added jcmd option gives users an option to dump a shared archive in one step at any time when the java application is running. Please check the CSR for more detail description for it. Thanks for your comment. Yumin
Re: RFR: 8258652: Assert in JvmtiThreadState::cur_stack_depth() can noticeably slow down debugging single stepping
On Fri, 18 Dec 2020 02:01:28 GMT, Chris Plummer wrote: > There is an assert in `JvmtiThreadState::cur_stack_depth()` that tends to > slow down single stepping a lot when running the debuggee with a debug jvm. > See CR for details. The fix is to allow disabling of this assert using the > new EnableJVMTIStackDepthAsserts global, which defaults to true. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1835
Re: RFR: 8259350: Add some internal debugging APIs to the debug agent
On Thu, 7 Jan 2021 04:28:19 GMT, Chris Plummer wrote: > This PR adds some APIs that are useful when debugging the debug agent. They > can be called from gdb or from other parts of the debug agent. They mostly do > things like dumping internal data structures that are tedious to dump or > iterate over in gdb. I developed them while working on loom and found them > useful. > > Note that `dumpThreadList()` and `dumpThread()` are not exported from > threadControl.c because the argument types are only visible within > threadControl.c. > > I debated whether to bracket all these APIs with `#ifdef DEBUG`. In the end I > decided to in order to make it clear they are only meant for debugging > purposes. If you temporarily need them with a product build, you can always > modify the code to include them. I could be persuaded `#ifdef DEBUG` though. Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1970
Re: RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect
On Thu, 14 Jan 2021 19:09:59 GMT, Leonid Mesnik wrote:
> est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of
> strcmp results here:
>
> for (i=0; i if (strcmp(methNam,METHODS[i][0]) &&
> strcmp(methSig,METHODS[i][1])) {
> printf("CHECK PASSED: method name: "%s"\tsignature: "%s" %d\n",
>methNam, methSig, i);
> if (checkStatus == PASSED)
> bpEvents[i]++;
> break;
> }
>
> So test passed when both strcmp (name,sig) are not zero.
>
> The test passes only because there are 2 methods that are checked and it
> increases counters for "incorrect" methods.
@lmesnik , looks good to me, besides a few nits.
test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp
line 2:
> 1: /*
> 2: * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights
> reserved.
it's 2021
test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp
line 173:
> 171:
> 172: for (i=0; i 173: if (strcmp(methNam,METHODS[i][0]) == 0 &&
could you please add space before `,`?
-
Marked as reviewed by iignatyev (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2084
RFR: 8259799: vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 is incorrect
est vmTestbase/nsk/jvmti/Breakpoint/breakpoint001 has incorrect check of strcmp results here: for (i=0; ihttps://git.openjdk.java.net/jdk/pull/2084/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2084=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259799 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2084.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2084/head:pull/2084 PR: https://git.openjdk.java.net/jdk/pull/2084
Re: RFR: 8252657: JVMTI agent is not unloaded when Agent_OnAttach is failed [v3]
On Thu, 14 Jan 2021 07:19:06 GMT, David Holmes wrote: > Hi Yasumasa, > > Spec update looks good. > > VM tweak to call dll_unload on failure looks good. Thanks! > I don't understand the test change though. To check the agent is unloaded, call VM.dynlibs dcmd and check whether agent path is not contained. libReturnError.so would be loaded only once in AttachReturnError.java, so it is expected to unload when `Agent_OnAttach()` fails. - PR: https://git.openjdk.java.net/jdk/pull/19
