date:20180411

Re: RFR: JDK-8174994: SA: clhsdb printmdo throws WrongTypeException when attached to a process with CDS

2018-04-11 Thread Jini George


Ping: Gentle reminder !

Thanks,
Jini.

On 4/6/2018 9:51 PM, Jini George wrote:

Hello!

Requesting reviews for: https://bugs.openjdk.java.net/browse/JDK-8174994
Webrev: http://cr.openjdk.java.net/~jgeorge/8174994/webrev.00/

While trying to identify the type given an address, a WrongTypeException 
was getting thrown with various clhsdb commands (like printmdo, jstack, 
etc). This was since SA tries to map an address to a hotspot C++ type by 
comparing the vtable address to the vtable address values of known 
types. With CDS, since the vtables are copied over for the Metadata 
classes, the vtable addresses themselves don't match (though, of course, 
the contents will), and SA errors out.


The fix has been implemented by making changes to read in the md region 
(consisting of the c++ vtables) of the CDS archive in SA, and mapping 
the vtable addresses to the corresponding metadata type (ConstantPool, 
InstanceKlass, InstanceClassLoaderKlass, InstanceMirrorKlass, 
InstanceRefKlass, Method, ObjArrayKlass, TypeArrayKlass).


For corefiles, an additional modification has been done to have the 
replicated FileMapHeader structure (from 
src/hotspot/share/memory/filemap.hpp, which is replicated in SA in 
ps_core.c), to be in sync with the corresponding definition in 
src/hotspot/share/memory/filemap.hpp.


Test cases to test both live and corefile debugging are being added with 
this. These and other SA tests pass on Mach5.


Thanks,
Jini.

Re: RFR(xxs): 8200384: jcmd help output should be sorted

2018-04-11 Thread Thomas Stüfe

Hi Serguei,

On Wed, Apr 11, 2018 at 7:51 PM, serguei.spit...@oracle.com
 wrote:
> Hi Thomas,
>
> Sorry, I did not reply to this.

no problem.

> Thank you a lot for providing the output!
> I agree, the sorted one looks much better.
>
> Probably, it is late to say but I'm Ok with a push. :)
>

Thanks :) Have not yet pushed but will do it today or tomorrow.

..Thomas

> Thanks,
> Serguei
>
>
>
> On 4/9/18 02:07, Thomas Stüfe wrote:
>
> Hi Sergey, Christoph,
>
> thanks for the review!
>
> Sure, here you go:
>
> Old output, unsorted:
>
> thomas@mainframe /shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
> ./images/jdk/bin/jcmd test3.Example2 help
> 24278:
> The following commands are available:
> VM.log
> VM.native_memory
> ManagementAgent.status
> ManagementAgent.stop
> ManagementAgent.start_local
> ManagementAgent.start
> Compiler.directives_clear
> Compiler.directives_remove
> Compiler.directives_add
> Compiler.directives_print
> Compiler.CodeHeap_Analytics
> VM.print_touched_methods
> Compiler.codecache
> Compiler.codelist
> Compiler.queue
> VM.classloader_stats
> Thread.print
> JVMTI.data_dump
> JVMTI.agent_load
> VM.metaspace
> VM.stringtable
> VM.symboltable
> VM.class_hierarchy
> VM.systemdictionary
> GC.class_stats
> GC.class_histogram
> GC.heap_dump
> GC.finalizer_info
> GC.heap_info
> GC.run_finalization
> GC.run
> VM.info
> VM.uptime
> VM.dynlibs
> VM.set_flag
> VM.flags
> VM.system_properties
> VM.command_line
> VM.version
> help
>
> New output, sorted:
>
> thomas@mainframe /shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
> ./images/jdk/bin/jcmd test3.Example2 help
> 30230:
> The following commands are available:
> Compiler.CodeHeap_Analytics
> Compiler.codecache
> Compiler.codelist
> Compiler.directives_add
> Compiler.directives_clear
> Compiler.directives_print
> Compiler.directives_remove
> Compiler.queue
> GC.class_histogram
> GC.class_stats
> GC.finalizer_info
> GC.heap_dump
> GC.heap_info
> GC.run
> GC.run_finalization
> JVMTI.agent_load
> JVMTI.data_dump
> ManagementAgent.start
> ManagementAgent.start_local
> ManagementAgent.status
> ManagementAgent.stop
> Thread.print
> VM.class_hierarchy
> VM.classloader_stats
> VM.command_line
> VM.dynlibs
> VM.flags
> VM.info
> VM.log
> VM.metaspace
> VM.native_memory
> VM.print_touched_methods
> VM.set_flag
> VM.stringtable
> VM.symboltable
> VM.system_properties
> VM.systemdictionary
> VM.uptime
> VM.version
> help
>
>
> I'm running submit tests now, if they pass I'll push.
>
> Best Regards, Thomas
>
>
> On Tue, Apr 3, 2018 at 3:52 AM, serguei.spit...@oracle.com
>  wrote:
>>
>> Hi Thomas,
>>
>> Added the serviceability-dev mailing list as it is a Serviceability area.
>>
>> The fix looks good to me.
>> One question:
>>  Could you, please, post the sorted help output?
>>  It is interesting how does it look like when sorted.
>>
>> Thanks,
>> Serguei
>>
>>
>>
>> On 3/28/18 13:08, Thomas Stüfe wrote:
>>>
>>> Hi all,
>>>
>>> may I get reviews for this tiny trivial change which causes jcmd help
>>> output (the command list) to be sorted?
>>>
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8200384
>>> webrev:
>>>
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8200384-jcmd-help-sorted/webrev.00/webrev/
>>>
>>> Thanks!
>>>
>>> Best Regards, Thomas
>>
>>
>
>

Re: RFR(xxs): 8200384: jcmd help output should be sorted

2018-04-11 Thread serguei.spit...@oracle.com


  
  
Hi Thomas,
  
  Sorry, I did not reply to this.
  Thank you a lot for providing the output!
  I agree, the sorted one looks much better.
  
  Probably, it is late to say but I'm Ok with a push. :)
  
  Thanks,
  Serguei
  
  
  On 4/9/18 02:07, Thomas Stüfe wrote:


  
  Hi Sergey, Christoph,


thanks for the review! 


Sure, here you go:


Old output, unsorted:



  thomas@mainframe
/shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
./images/jdk/bin/jcmd test3.Example2 help
  24278:
  The following commands are available:
  VM.log
  VM.native_memory
  ManagementAgent.status
  ManagementAgent.stop
  ManagementAgent.start_local
  ManagementAgent.start
  Compiler.directives_clear
  Compiler.directives_remove
  Compiler.directives_add
  Compiler.directives_print
  Compiler.CodeHeap_Analytics
  VM.print_touched_methods
  Compiler.codecache
  Compiler.codelist
  Compiler.queue
  VM.classloader_stats
  Thread.print
  JVMTI.data_dump
  JVMTI.agent_load
  VM.metaspace
  VM.stringtable
  VM.symboltable
  VM.class_hierarchy
  VM.systemdictionary
  GC.class_stats
  GC.class_histogram
  GC.heap_dump
  GC.finalizer_info
  GC.heap_info
  GC.run_finalization
  GC.run
  VM.info
  VM.uptime
  VM.dynlibs
  VM.set_flag
  VM.flags
  VM.system_properties
  VM.command_line
  VM.version
  help



New output, sorted:



  thomas@mainframe
/shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
./images/jdk/bin/jcmd test3.Example2 help
  30230:
  The following commands are available:
  Compiler.CodeHeap_Analytics
  Compiler.codecache
  Compiler.codelist
  Compiler.directives_add
  Compiler.directives_clear
  Compiler.directives_print
  Compiler.directives_remove
  Compiler.queue
  GC.class_histogram
  GC.class_stats
  GC.finalizer_info
  GC.heap_dump
  GC.heap_info
  GC.run
  GC.run_finalization
  JVMTI.agent_load
  JVMTI.data_dump
  ManagementAgent.start
  ManagementAgent.start_local
  ManagementAgent.status
  ManagementAgent.stop
  Thread.print
  VM.class_hierarchy
  VM.classloader_stats
  VM.command_line
  VM.dynlibs
  VM.flags
  VM.info
  VM.log
  VM.metaspace
  VM.native_memory
  VM.print_touched_methods
  VM.set_flag
  VM.stringtable
  VM.symboltable
  VM.system_properties
  VM.systemdictionary
  VM.uptime
  VM.version
  help





I'm running submit tests now, if they pass I'll push.


Best Regards, Thomas


  
  
On Tue, Apr 3, 2018 at 3:52 AM, serguei.spit...@oracle.com 
  wrote:
  Hi Thomas,

Added the serviceability-dev mailing list as it is a
Serviceability area.

The fix looks good to me.
One question:
 Could you, please, post the sorted help output?
 It is interesting how does it look like when sorted.

Thanks,
Serguei

  


On 3/28/18 13:08, Thomas Stüfe wrote:

  Hi all,
  
  may I get reviews for this tiny trivial change which
  causes jcmd help
  output (the command list) to be sorted?
  
  bug: https://bugs.openjdk.java.net/browse/JDK-8200384
  webrev:
  http://cr.openjdk.java.net/~stuefe/webrevs/8200384-jcmd-help-sorted/webrev.00/webrev/
  
  Thanks!
  
  Best Regards, Thomas

Re: RFR(xxs): 8200384: jcmd help output should be sorted

2018-04-11 Thread serguei.spit...@oracle.com


  
  
Hi Thomas,
  
  Sorry, I did not reply to this.
  Thank you a lot for providing the output!
  I agree, the sorted one looks much better.
  
  Probably, it is late to say but I'm Ok with a push. :)
  
  Thanks,
  Serguei
  
  
  On 4/9/18 02:07, Thomas Stüfe wrote:


  
  Hi Sergey, Christoph,


thanks for the review! 


Sure, here you go:


Old output, unsorted:



  thomas@mainframe
/shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
./images/jdk/bin/jcmd test3.Example2 help
  24278:
  The following commands are available:
  VM.log
  VM.native_memory
  ManagementAgent.status
  ManagementAgent.stop
  ManagementAgent.start_local
  ManagementAgent.start
  Compiler.directives_clear
  Compiler.directives_remove
  Compiler.directives_add
  Compiler.directives_print
  Compiler.CodeHeap_Analytics
  VM.print_touched_methods
  Compiler.codecache
  Compiler.codelist
  Compiler.queue
  VM.classloader_stats
  Thread.print
  JVMTI.data_dump
  JVMTI.agent_load
  VM.metaspace
  VM.stringtable
  VM.symboltable
  VM.class_hierarchy
  VM.systemdictionary
  GC.class_stats
  GC.class_histogram
  GC.heap_dump
  GC.finalizer_info
  GC.heap_info
  GC.run_finalization
  GC.run
  VM.info
  VM.uptime
  VM.dynlibs
  VM.set_flag
  VM.flags
  VM.system_properties
  VM.command_line
  VM.version
  help



New output, sorted:



  thomas@mainframe
/shared/projects/openjdk/jdk-submit-hs/output-fastdebug $
./images/jdk/bin/jcmd test3.Example2 help
  30230:
  The following commands are available:
  Compiler.CodeHeap_Analytics
  Compiler.codecache
  Compiler.codelist
  Compiler.directives_add
  Compiler.directives_clear
  Compiler.directives_print
  Compiler.directives_remove
  Compiler.queue
  GC.class_histogram
  GC.class_stats
  GC.finalizer_info
  GC.heap_dump
  GC.heap_info
  GC.run
  GC.run_finalization
  JVMTI.agent_load
  JVMTI.data_dump
  ManagementAgent.start
  ManagementAgent.start_local
  ManagementAgent.status
  ManagementAgent.stop
  Thread.print
  VM.class_hierarchy
  VM.classloader_stats
  VM.command_line
  VM.dynlibs
  VM.flags
  VM.info
  VM.log
  VM.metaspace
  VM.native_memory
  VM.print_touched_methods
  VM.set_flag
  VM.stringtable
  VM.symboltable
  VM.system_properties
  VM.systemdictionary
  VM.uptime
  VM.version
  help





I'm running submit tests now, if they pass I'll push.


Best Regards, Thomas


  
  
On Tue, Apr 3, 2018 at 3:52 AM, serguei.spit...@oracle.com 
  wrote:
  Hi Thomas,

Added the serviceability-dev mailing list as it is a
Serviceability area.

The fix looks good to me.
One question:
 Could you, please, post the sorted help output?
 It is interesting how does it look like when sorted.

Thanks,
Serguei

  


On 3/28/18 13:08, Thomas Stüfe wrote:

  Hi all,
  
  may I get reviews for this tiny trivial change which
  causes jcmd help
  output (the command list) to be sorted?
  
  bug: https://bugs.openjdk.java.net/browse/JDK-8200384
  webrev:
  http://cr.openjdk.java.net/~stuefe/webrevs/8200384-jcmd-help-sorted/webrev.00/webrev/
  
  Thanks!
  
  Best Regards, Thomas

Re: RFR: Fix race condition in jdwp

2018-04-11 Thread Andrew Leonard

Hi Serguei,
Thank you for raising the bug.
I had a chat with one of my colleagues who could recreate it, and it's 
probably related to the handshaking that is done in the particular 
scenario. So with the JCK harness:

com.sun.jck.lib.ExecJCKTestOtherJVMCmd LD_LIBRARY_PATH=/javatest/lib/jck
/jck8b/natives/linux_x86-64 /projects/jck/jdwp/j2sdk-image/bin/java 
-Xdump:system:none -Xdump:system:events=gpf+abort+traceassert+corruptcache 
-Xdump:snap:none -Xdump:snap:events=gpf+abort+traceassert+corruptcache 
-Xdump:java:none -Xdump:java:events=gpf+abort+traceassert+corruptcache 
-Xdump:heap:none -Xdump:heap:events=gpf+abort+traceassert+corruptcache -
Xfuture -agentlib:jdwp=server=y,transport=dt_socket,address=localhost
:35000,suspend=y -classpath /javatest/lib/jck
/JCK8b-b03/JCK-runtime-8b/classes -Djava.security.policy=/javatest/lib/jck
/JCK8b-b03/JCK-runtime-8b/lib/jck.policy 
javasoft.sqe.jck.lib.jpda.jdwp.DebuggeeLoader -waittime=600 
-msgSwitch=ub1604x64vm10:38636 -componentName=
ArrayReference.GetValues.getvalues002

Note that the JCK test harness starts the target process, attaches to it, 
and sends the resume command 
in a very short time with no handshaking.

That may not help..but hopefully helps explain things a bit? It's the 
timing of the resume command during the test that is crucial, resuming 
before the VM initialization is complete will trigger it.

Thanks
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

From:   "serguei.spit...@oracle.com" 
To: Andrew Leonard 
Cc: serviceability-dev@openjdk.java.net
Date:   11/04/2018 09:57
Subject:Re: RFR: Fix race condition in jdwp

Hi Andrew,

I've filed the bug:
  https://bugs.openjdk.java.net/browse/JDK-8201409

Also, this is a webrev with your patch:

http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8201409-jdwp-initsync.ibm.1/

I agree that creating a standalone test is tricky here.

I've added usleep(1) into the eventHelper_reportVMInit()
and ran the JTreg com/sun/jdi tests with my JDK build.
However, none of the tests failed with the failure mode you described.
So that I'm puzzled a little bit.
I suspect that some specific debugLoop commands were used in your 
scenario.

It is still possible that I've missed something here.
Will try to double check everything.

Thanks,
Serguei

On 4/11/18 01:29, Andrew Leonard wrote:
Thanks Serguei, 
I terms of a standalone testcase it is quite tricky, as due to the nature 
of the issue which took a lot of investigation to solve it's very timing 
dependent and will only occur randomly. It can be forced as I indicated 
below by adding a "sleep" in the VMInit report code but that's not a 
testcase, however the issue was originally found in our JCK testing for 
IBMJava8, testcase test.jck8b.runtime.vm.jdwp, but again only happened 
intermittently. Sort of like "performance" type issues we're not always 
going to be able to create a testcase that will always "fail" if the fix 
is not present. 
Your thoughts? 
Cheers 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

From:"serguei.spit...@oracle.com"  
To:Andrew Leonard  
Cc:serviceability-dev@openjdk.java.net 
Date:11/04/2018 01:02 
Subject:Re: RFR: Fix race condition in jdwp 

Hi Andrew,

Okay, I'll file a bug on this topic.
But do you have a standalone test demonstrating this issue?

Thanks,
Serguei

On 4/10/18 06:23, Andrew Leonard wrote: 
Hi Serguei, 
I don't have access to the bug database to raise one, are you able to 
please? 

Summary: JDWP debugger initialization hangs intermittently 
Description: If during the JDWP setup initialization the VM initialization 
takes slightly longer than the main debug initialization thread a "hang" 
situation can occur. This has been seen in testcase 
test.jck8b.runtime.vm.jdwp and can also be recreated easily by adding a 10 
second sleep to the beginning of the 
src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c method 
eventHelper_reportVMInit() . 
First seen: JDK8 
Recreated: JDK11 

Thanks 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

From:"serguei.spit...@oracle.com"  
To:Andrew Leonard , 
serviceability-dev@openjdk.java.net 
Date:09/04/2018 23:03 
Subject:Re: RFR: Fix race condition in jdwp 

Hi Andrew,

The patch itself looks reasonable.
However, in order to proceed with it, a bug report with a standalone
test case demonstrating the issue is needed.

Thanks,
Serguei

On 4/9/18 09:07, Andrew Leonard wrote:
> Hi,
> We discovered in our testing with OpenJ9 that a race condition can 
> occur in the jdwp under certain circumstances,

Re: RFR: Fix race condition in jdwp

2018-04-11 Thread serguei.spit...@oracle.com

Hi Andrew,

  I've filed the bug:
    https://bugs.openjdk.java.net/browse/JDK-8201409

  Also, this is a webrev with your patch:

http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8201409-jdwp-initsync.ibm.1/

  I agree that creating a standalone test is tricky here.

  I've added usleep(1) into the eventHelper_reportVMInit()
  and ran the JTreg com/sun/jdi tests with my JDK build.
  However, none of the tests failed with the failure mode you
  described.
  So that I'm puzzled a little bit.
  I suspect that some specific debugLoop commands were used in your
  scenario.

  It is still possible that I've missed something here.
  Will try to double check everything.

  Thanks,
  Serguei

  On 4/11/18 01:29, Andrew Leonard wrote:

  Thanks Serguei,

  I terms of a standalone
testcase it is quite tricky, as due to the nature of the issue
which took
a lot of investigation to solve it's very timing dependent and
will only
occur randomly. It can be forced as I indicated below by adding
a "sleep"
in the VMInit report code but that's not a testcase, however the
issue
was originally found in our JCK testing for IBMJava8, testcase test.jck8b.runtime.vm.jdwp,
but again only happened intermittently. Sort of like
"performance"
type issues we're not always going to be able to create a
testcase that
will always "fail" if the fix is not present.

  Your thoughts?

  Cheers

  Andrew

  Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

  From:
       "serguei.spit...@oracle.com"

  To:
       Andrew
Leonard 

  Cc:
       serviceability-dev@openjdk.java.net

  Date:
       11/04/2018
01:02

  Subject:
       Re:
RFR: Fix race condition in jdwp

  Hi Andrew,

Okay, I'll file a bug on this topic.
But do you have a standalone test demonstrating this issue?

Thanks,
Serguei

On 4/10/18 06:23, Andrew Leonard wrote:

  Hi Serguei, 
I don't have access to the bug database to raise one, are you
able to please?

Summary: JDWP debugger initialization hangs intermittently 
Description: If during the JDWP setup initialization the VM
initialization
takes slightly longer than the main debug initialization thread
a "hang"
situation can occur. This has been seen in testcase
test.jck8b.runtime.vm.jdwp
and can also be recreated easily by adding a 10 second sleep to
the beginning
of the src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c
method eventHelper_reportVMInit()
. 
First seen: JDK8 
Recreated: JDK11 

Thanks 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com

From:        "serguei.spit...@oracle.com"

To:        Andrew Leonard ,
  serviceability-dev@openjdk.java.net

Date:        09/04/2018 23:03 
Subject:        Re: RFR: Fix race condition in jdwp

  Hi Andrew,

  The patch itself looks reasonable.
  However, in order to proceed with it, a bug report with a
  standalone
  test case demonstrating the issue is needed.

  Thanks,
  Serguei

  On 4/9/18 09:07, Andrew Leonard wrote:
  > Hi,
  > We discovered in our testing with OpenJ9 that a race
  condition can

  > occur in the jdwp under certain circumstances, and we
  were able to

  > force the same issue with Hotspot. Normally, the event
  helper thread

  > suspends all threads, then the debug loop in the listener
  thread 
  > receives a command to resume. The debugger may deadlock
  if the debug

  > loop in the listener thread starts processing commands
  (e.g. resume

  > threads) before the event helper completes the
  initialization (and

  > suspends threads).

Re: RFR: Fix race condition in jdwp

2018-04-11 Thread Andrew Leonard

Thanks Serguei,
I terms of a standalone testcase it is quite tricky, as due to the nature 
of the issue which took a lot of investigation to solve it's very timing 
dependent and will only occur randomly. It can be forced as I indicated 
below by adding a "sleep" in the VMInit report code but that's not a 
testcase, however the issue was originally found in our JCK testing for 
IBMJava8, testcase test.jck8b.runtime.vm.jdwp, but again only happened 
intermittently. Sort of like "performance" type issues we're not always 
going to be able to create a testcase that will always "fail" if the fix 
is not present.
Your thoughts?
Cheers
Andrew

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

From:   "serguei.spit...@oracle.com" 
To: Andrew Leonard 
Cc: serviceability-dev@openjdk.java.net
Date:   11/04/2018 01:02
Subject:Re: RFR: Fix race condition in jdwp

Hi Andrew,

Okay, I'll file a bug on this topic.
But do you have a standalone test demonstrating this issue?

Thanks,
Serguei

On 4/10/18 06:23, Andrew Leonard wrote:
Hi Serguei, 
I don't have access to the bug database to raise one, are you able to 
please? 

Summary: JDWP debugger initialization hangs intermittently 
Description: If during the JDWP setup initialization the VM initialization 
takes slightly longer than the main debug initialization thread a "hang" 
situation can occur. This has been seen in testcase 
test.jck8b.runtime.vm.jdwp and can also be recreated easily by adding a 10 
second sleep to the beginning of the 
src/jdk.jdwp.agent/share/native/libjdwp/eventHelper.c method 
eventHelper_reportVMInit() . 
First seen: JDK8 
Recreated: JDK11 

Thanks 
Andrew 

Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 

From:"serguei.spit...@oracle.com"  
To:Andrew Leonard , 
serviceability-dev@openjdk.java.net 
Date:09/04/2018 23:03 
Subject:Re: RFR: Fix race condition in jdwp 

Hi Andrew,

The patch itself looks reasonable.
However, in order to proceed with it, a bug report with a standalone
test case demonstrating the issue is needed.

Thanks,
Serguei

On 4/9/18 09:07, Andrew Leonard wrote:
> Hi,
> We discovered in our testing with OpenJ9 that a race condition can 
> occur in the jdwp under certain circumstances, and we were able to 
> force the same issue with Hotspot. Normally, the event helper thread 
> suspends all threads, then the debug loop in the listener thread 
> receives a command to resume. The debugger may deadlock if the debug 
> loop in the listener thread starts processing commands (e.g. resume 
> threads) before the event helper completes the initialization (and 
> suspends threads).
>
> This patch adds synchronization to ensure the event helper completes 
> the initialization sequence before debugger commands are processed.
>
> Please can I find a sponsor for this contribution? Patch below..
>
> Many thanks
>
> Andrew
>
>
>
> diff --git a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c 
> b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> --- a/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> +++ b/src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights 
> reserved.
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>   *
>   * This code is free software; you can redistribute it and/or modify it
> @@ -58,6 +58,7 @@
>  static jboolean vmInitialized;
>  static jrawMonitorID initMonitor;
>  static jboolean initComplete;
> +static jboolean VMInitComplete;
>  static jbyte currentSessionID;
>
>  /*
> @@ -617,6 +618,35 @@
>  debugMonitorExit(initMonitor);
>  }
>
> +/*
> + * Signal VM initialization is complete.
> + */
> +void
> +signalVMInitComplete(void)
> +{
> +/*
> + * VM Initialization is complete
> + */
> +LOG_MISC(("signal VM initialization complete"));
> +debugMonitorEnter(initMonitor);
> +VMInitComplete = JNI_TRUE;
> +debugMonitorNotifyAll(initMonitor);
> +debugMonitorExit(initMonitor);
> +}
> +
> +/*
> + * Wait for VM initialization to complete.
> + */
> +void
> +debugInit_waitVMInitComplete(void)
> +{
> +debugMonitorEnter(initMonitor);
> +while (!VMInitComplete) {
> +debugMonitorWait(initMonitor);
> +}
> +debugMonitorExit(initMonitor);
> +}
> +
>  /* All process exit() calls come from here */
>  void
>  forceExit(int exit_code)
> @@ -672,6 +702,7 @@
>  LOG_MISC(("Begin initialize()"));
>  currentSessionID = 0;
>  initComplete = JNI_FALSE;
> +VMInitComplete = JNI_FALSE;
>
>  if ( gdata->vmDead ) {
>  EXIT_ERROR(AGENT_ERROR_INTERNAL,"VM dead at initialize() time");
> diff --git a/src/jd

RE: RFR (M): 8201247: Various cleanups in the attach framework

2018-04-11 Thread Langer, Christoph

Hi Goetz,

I did not aim to sort the includes in the attachListener_.cpp files. I also 
just saw that I made a change in attachListener_aix.cpp for some other 
merge-related reasons - but it was not correctly sorted before and neither is 
after 😉 This has to be done in a future change...

Best regards
Christoph

> -Original Message-
> From: Lindenmaier, Goetz
> Sent: Mittwoch, 11. April 2018 09:56
> To: Langer, Christoph ; serviceability-
> d...@openjdk.java.net
> Cc: hotspot-...@openjdk.java.net
> Subject: RE: RFR (M): 8201247: Various cleanups in the attach framework
> 
> Hi Christoph,
> 
> I'm familiar with the non-system includes in hotspot,
> there mostly a total alphabetical ordering is followed, like in
> http://hg.openjdk.java.net/jdk/hs/file/0d8ed8b2ac4f/src/hotspot/cpu/x86/
> c1_CodeStubs_x86.cpp
> > It is. However, I have put "subdirs" first. That is, the includes from sys/*
> Also, that's not true, see the first file in your webrev, which is sorted as I
> would expect:
> http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/hotspot/os/aix
> /attachListener_aix.cpp.html
> while this sorts as you state:
> http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/jdk.attach/linux
> /native/libattach/VirtualMachineImpl.c.html
> 
> Is there a different pattern followed in hotspot and jdk coding?
> 
> In case you resort them (have fun :) ), no new webrev is needed.
> 
> Best regards,
>   Goetz.
> 
> 
> > -Original Message-
> > From: Langer, Christoph
> > Sent: Mittwoch, 11. April 2018 09:45
> > To: Lindenmaier, Goetz ; serviceability-
> > d...@openjdk.java.net
> > Cc: hotspot-...@openjdk.java.net
> > Subject: RE: RFR (M): 8201247: Various cleanups in the attach framework
> >
> > Hi Goetz,
> >
> > thanks for the review.
> >
> > > You say you are sorting the includes, but in the VirtualMachineImpl.c
> > > files the order is changed, but according to which order? It's
> > > not alphabetical as in other files.
> >
> > It is. However, I have put "subdirs" first. That is, the includes from sys/*
> > come first (in alphabetical order).
> >
> > Best regards
> > Christoph

RE: RFR (M): 8201247: Various cleanups in the attach framework

2018-04-11 Thread Lindenmaier, Goetz

Hi Christoph, 

I'm familiar with the non-system includes in hotspot, 
there mostly a total alphabetical ordering is followed, like in 
http://hg.openjdk.java.net/jdk/hs/file/0d8ed8b2ac4f/src/hotspot/cpu/x86/c1_CodeStubs_x86.cpp
> It is. However, I have put "subdirs" first. That is, the includes from sys/*
Also, that's not true, see the first file in your webrev, which is sorted as I 
would expect:
http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/hotspot/os/aix/attachListener_aix.cpp.html
while this sorts as you state:
http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/jdk.attach/linux/native/libattach/VirtualMachineImpl.c.html

Is there a different pattern followed in hotspot and jdk coding?

In case you resort them (have fun :) ), no new webrev is needed.

Best regards,
  Goetz.


> -Original Message-
> From: Langer, Christoph
> Sent: Mittwoch, 11. April 2018 09:45
> To: Lindenmaier, Goetz ; serviceability-
> d...@openjdk.java.net
> Cc: hotspot-...@openjdk.java.net
> Subject: RE: RFR (M): 8201247: Various cleanups in the attach framework
> 
> Hi Goetz,
> 
> thanks for the review.
> 
> > You say you are sorting the includes, but in the VirtualMachineImpl.c
> > files the order is changed, but according to which order? It's
> > not alphabetical as in other files.
> 
> It is. However, I have put "subdirs" first. That is, the includes from sys/*
> come first (in alphabetical order).
> 
> Best regards
> Christoph

RE: RFR (M): 8201247: Various cleanups in the attach framework

2018-04-11 Thread Langer, Christoph

Thanks Chris, I'll push it then today.

> -Original Message-
> From: Chris Plummer [mailto:chris.plum...@oracle.com]
> Sent: Mittwoch, 11. April 2018 07:37
> To: Langer, Christoph ; serviceability-
> d...@openjdk.java.net
> Cc: hotspot-...@openjdk.java.net
> Subject: Re: RFR (M): 8201247: Various cleanups in the attach framework
> 
> Hi Christoph,
> 
> I finished testing. No issues.
> 
> thanks,
> 
> Chris
> 
> On 4/10/18 1:08 PM, Chris Plummer wrote:
> > Hi Christoph,
> >
> > I'm somewhat new to looking at submit-hs test jobs. However I see know
> > indication of there being a submit for JDK-8201247, so I don't think
> > it was run. I'll start my own testing with the last patch you sent out.
> >
> > thanks,
> >
> > Chris
> >
> > On 4/10/18 1:01 PM, Langer, Christoph wrote:
> >> Hi Chris,
> >>
> >> I ran the jtreg tests under hotspot/jtreg/serviceability/attach and
> >> jdk/com/sun/tools/attach for the main platforms (Windows, Linux
> >> X86_64, mac, solaris and AIX).
> >>
> >> I also pushed to submit-hs in branch "JDK-8201247" but it seems I
> >> have no luck and got no notification mails. May I ask you to check
> >> whether the build/test cycle was run and how the results looked like?
> >>
> >> Please also do your closed testing and let me know the outcome.
> >>
> >> Thanks a lot in advance
> >> Christoph
> >>
> >>> -Original Message-
> >>> From: Chris Plummer [mailto:chris.plum...@oracle.com]
> >>> Sent: Montag, 9. April 2018 20:05
> >>> To: Langer, Christoph ; serviceability-
> >>> d...@openjdk.java.net
> >>> Cc: hotspot-...@openjdk.java.net
> >>> Subject: Re: RFR (M): 8201247: Various cleanups in the attach framework
> >>>
> >>> Hi Christoph,
> >>>
> >>> We have some closed "attach on demand" tests that should be run also.
> I
> >>> can do this for you when you are ready. Please also let me know which
> >>> other jtreg tests you have run.
> >>>
> >>> thanks,
> >>>
> >>> Chris
> >>>
> >>> On 4/9/18 12:08 AM, Langer, Christoph wrote:
>  Hi Chris,
> 
>  thanks for looking into this.
> 
>  As for ArgumentIterator::next, I must admit, I found this patch in
>  our code
> >>> base when taking over the code. I believe that an issue would be
> >>> seen if an
> >>> attach operation has 2 or 3 arguments and the first one is
> >>> NULL/empty. I
> >>> guess such a situation can't happen with the attach operations
> >>> currently
> >>> existing in OpenJDK as none of these ops would allow such type of
> >>> arguments. However, in our implementation, we have for instance
> >>> enhanced
> >>> the "dump_heap" operation to work with null as first argument where
> one
> >>> usually would specify the desired output file name. We implemented a
> >>> mechanism to compute a default filename when the param is left
> >>> blank. So
> >>> we need the fix for that case, I guess.
>  I'll run the patch through the submission forest now and do some jtreg
> >>> testing.
>  Best regards
>  Christoph
> 
> > -Original Message-
> > From: Chris Plummer [mailto:chris.plum...@oracle.com]
> > Sent: Freitag, 6. April 2018 18:37
> > To: Langer, Christoph ; serviceability-
> > d...@openjdk.java.net
> > Cc: hotspot-...@openjdk.java.net
> > Subject: Re: RFR (M): 8201247: Various cleanups in the attach
> > framework
> >
> > Hi Christoph,
> >
> > Can you explain a bit more about "fix handling of null values in
> > ArgumentIterator::next". When does this turn up? Is there a test
> > case?
> >
> > Everything else looks good.
> >
> > thanks,
> >
> > Chris
> >
> > On 4/6/18 8:01 AM, Langer, Christoph wrote:
> >> Hi,
> >>
> >> can I please get reviews for a set of clean up changes that I came
> >> across when doing some integration work.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8201247
> >> 
> >>
> >> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/
> >> 
> >>
> >> Detailed comments about the changes can be found in the bug.
> >>
> >> Thanks & best regards
> >>
> >> Christoph
> >>
> >

RE: RFR (M): 8201247: Various cleanups in the attach framework

2018-04-11 Thread Langer, Christoph

Hi Goetz,

thanks for the review.

> You say you are sorting the includes, but in the VirtualMachineImpl.c
> files the order is changed, but according to which order? It's
> not alphabetical as in other files.

It is. However, I have put "subdirs" first. That is, the includes from sys/* 
come first (in alphabetical order).

Best regards
Christoph

Re: RFR: JDK-8174994: SA: clhsdb printmdo throws WrongTypeException when attached to a process with CDS

Re: RFR(xxs): 8200384: jcmd help output should be sorted

Re: RFR(xxs): 8200384: jcmd help output should be sorted

Re: RFR(xxs): 8200384: jcmd help output should be sorted

Re: RFR: Fix race condition in jdwp

Re: RFR: Fix race condition in jdwp

Re: RFR: Fix race condition in jdwp

RE: RFR (M): 8201247: Various cleanups in the attach framework

RE: RFR (M): 8201247: Various cleanups in the attach framework

RE: RFR (M): 8201247: Various cleanups in the attach framework

RE: RFR (M): 8201247: Various cleanups in the attach framework

11 matches

Site Navigation

Mail list logo

Footer information