date:20180207


On 07/02/2018 09:19, Harsha Wardhana B wrote:

Hi All,

After internal discussions, below format for management flags was 
agreed upon.


1. --start-management-agent port=1234,ssl=on        (space seperator)
or
2. --start-management-agent=port=1234,ssl=on        ('=' seperator)

If option 1 is specified, it will be converted to option 2 by the java 
launcher before it is passed onto VM.


With above GNU long format for management options, specifying 
arguments is mandatory unlike before.


--start-management-agent will not be recognized in the current format 
and hence will not default to --start-management-agent=local=true.
`--start-management-agent ` make sense and it wouldn't be hard to 
introduce `--start-local-management-agent` if really needed.


One point that needs discussion is whether the  needs something, 
maybe an agent name or kind, to make it clear what agent should be 
started. Today we have a JMX management agent, Oracle JDK downloads 
additionally have a SNMP management agent. There are several 3rd party 
exporters. If AgentProvider were promoted to an exported package then it 
would make this pluggibility a lot cleaner and would mean the 
--start-management-agent could specify the agent name/kind without 
needing hacks or forwarding by the JMX agent.


-Alan

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()


On 07/02/2018 18:02, gary.ad...@oracle.com wrote:

Yes, WSASendDisconnect is deprecated in vs2013.
As far as I know "shutdown(fd, SD_SEND)" prevents further outgoing writes
and there was no final payload to send.

I have not seen any failures in the java/nio tests.

Okay, I guess it should okay but can you fix the formatting before you 
push this? (8 space indent should be 4).


Also we need to fix the bug information as core-svc/debugger and a 
summary saying it's debugger transport issue is misleading. You've done 
the right thing of course to fix the larger issue and the bug should 
reflect that.


-Alan

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

2018-02-07 Thread Harsha Wardhana B

Hi Kumar,

Thanks for your inputs.

On Wednesday 07 February 2018 09:02 PM, Kumar Srinivasan wrote:

Hi Harsha,

Changes look reasonable to me, couple of things that must be addressed:

1. Since this is a main-stream launcher change with a documented and
supported
option, a CSR is required, you have to add and document the option
in the help page
http://hg.openjdk.java.net/jdk/jdk/file/8cc67294ec56/src/java.base/share/classes/sun/launcher/resources/launcher.properties

2. You also have to create a doc bug so that the doc team will
document it in the Tools
Reference Guide, and link it to this bug. Does it need a Release note
? probably does,
in which case you will have to create Release Note subtask and follow
the RN process.

Will take care of above items.

3. Is XmanagementAgentTest.java part of tier1 test suite ? If not,
then I think it ought
to be in tier1 grouping, perhaps best to park this under
jdk/tools/launcher/management ?

Ok. I will move the test in subsequent webrev.

-Harsha

Kumar

Hi All,

After internal discussions, below format for management flags was
agreed upon.

1. --start-management-agent port=1234,ssl=on (space seperator)
or
2. --start-management-agent=port=1234,ssl=on ('=' seperator)

If option 1 is specified, it will be converted to option 2 by the
java launcher before it is passed onto VM.

With above GNU long format for management options, specifying
arguments is mandatory unlike before.

--start-management-agent will not be recognized in the current format
and hence will not default to --start-management-agent=local=true.

Below is the webrev with above changes and corresponding tests.

http://cr.openjdk.java.net/~hb/8187498/webrev.01/

Please review and comment.

Thanks
Harsha

On Monday 29 January 2018 03:14 PM, Harsha Wardhana B wrote:

Hi Alan,

I am not fully aware about Java launcher or how it passes options to
VM. Let me check with some other folks and get back to you.

Thanks
Harsha

On Monday 29 January 2018 01:55 PM, Alan Bateman wrote:

On 29/01/2018 05:20, Harsha Wardhana B wrote:

Hi Mandy,Alan,

Thanks for your inputs.
If I keep it as launcher option, it may need to know JMX agent
flags which may need to be extended in future.
I would prefer making it a VM option. I will make the required
changes and send out an updated webrev.
I think Mandy's suggestion is to just transform --management
so a form that the VM can read. The launcher will need to
replace the space anyway as the VM only accepts "=".

-Alan

Re: Issue with 8192897: NPE occurs on clhsdb jstack

2018-02-07 Thread Jini George

Thanks for this, Daniel. Is this consistently reproducible on the 
Aarch64 system with the -Xcomp part of the test ?


Thanks,
Jini

On 2/7/2018 10:12 PM, stewartd.qdt wrote:
I am getting Null Pointer Exceptions with both ClhsdbJstack.java and 
ClhsdbFindPC.java. It appears the addition of testing with  ‘-Xcomp` 
causing a Method to come back as null during the stack walk. I have 
included the results of the Jstack test (the FindPC is similar) for the 
Xcomp section below. I have created bug JDK-8196969 
.  The full .jtr is 
attached there.  I am not familiar enough with the Java stack to know if 
having  a null Method pointer is valid or not.


Thanks,

Daniel

hsdb>

Command line: 
['/home/stewartd/openjdk/OpenJDK/hs/build/linux-aarch64-normal-server-release/images/jdk/bin/java' 
'-Xcomp' '-cp' 
'/home/stewartd/openjdk/OpenJDK/hs/JTwork/classes/serviceability/sa/ClhsdbJstack.d:/home/stewartd/openjdk/OpenJDK/hs/JTwork/classes/test/lib' 
'jdk.test.lib.apps.LingeredApp' '717a6b55-bcf8-405e-9d5a-afe1212f0fc6.lck' ]


Started LingeredApp (-Xcomp) with pid 23731

Starting clhsdb against 23731

Warning! JS Engine can't start, some commands will not be available.

hsdb> Deadlock Detection:

No deadlocks found.

"Common-Cleaner" #22 daemon prio=8 tid=0xa0528000 nid=0x5d2d in 
Object.wait() [0xfffe427de000]


    java.lang.Thread.State: TIMED_WAITING (on object monitor)

    JavaThread state: _thread_blocked

- java.lang.ref.ReferenceQueue.remove(long) @bci=59, line=151, 
pc=0x892af7c4, Method*=0xfffe49851a98 (Compiled frame; 
information may be imprecise)


     - waiting to lock <0x000101c956f0> (a 
java.lang.ref.ReferenceQueue$Lock)


- jdk.internal.ref.CleanerImpl.run() @bci=65, line=148, 
pc=0x892ac8e4, Method*=0xfffe49ab23d8 (Compiled frame)


- java.lang.Thread.run() @bci=11, line=844, pc=0x892a9d30, 
Method*=0xfffe49738ee0 (Compiled frame)


- jdk.internal.misc.InnocuousThread.run() @bci=20, line=134, 
pc=0x892a9d30, Method*=0xfffe49aba318 (Compiled frame)


Locked ownable synchronizers:

     - None

"Signal Dispatcher" #4 daemon prio=9 tid=0xa04a6800 nid=0x5d05 
runnable [0x]


    java.lang.Thread.State: RUNNABLE

    JavaThread state: _thread_blocked

Locked ownable synchronizers:

     - None

"Finalizer" #3 daemon prio=8 tid=0xa0494000 nid=0x5d04 in 
Object.wait() [0xfffe492ee000]


    java.lang.Thread.State: WAITING (on object monitor)

    JavaThread state: _thread_blocked

- java.lang.ref.ReferenceQueue.remove(long) @bci=59, line=151, 
pc=0x888e15a0, Method*=0xfffe49851a98 (Interpreted frame)


     - waiting to lock <0x000101c09528> (a 
java.lang.ref.ReferenceQueue$Lock)


- java.lang.ref.ReferenceQueue.remove() @bci=2, line=172, 
pc=0x888e12c0, Method*=0xfffe49851b40 (Interpreted frame)


- java.lang.ref.Finalizer$FinalizerThread.run() @bci=37, line=216, 
pc=0x888e12c0, Method*=0xfffe49856cb0 (Interpreted frame)


Locked ownable synchronizers:

     - None

"Reference Handler" #2 daemon prio=10 tid=0xa0492000 nid=0x5d03 
waiting on condition [0xfffe494ee000]


    java.lang.Thread.State: RUNNABLE

    JavaThread state: _thread_blocked

- java.lang.ref.Reference.processPendingReferences() @bci=0, line=166, 
pc=0x888e15a0, Method*=0xfffe49733100 (Interpreted frame)


- java.lang.ref.Reference.access$000() @bci=0, line=44, 
pc=0x888e15a0, Method*=0xfffe49733788 (Interpreted frame)


- java.lang.ref.Reference$ReferenceHandler.run() @bci=0, line=138, 
pc=0x888e15a0, Method*=0xfffe4984ffa0 (Interpreted frame)


Locked ownable synchronizers:

     - None

"main" #1 prio=5 tid=0xa001 nid=0x5cb5 waiting on condition 
[0xa5d2e000]


    java.lang.Thread.State: TIMED_WAITING (sleeping)

    JavaThread state: _thread_blocked

Error occurred during stack walking:

Locked ownable synchronizers:

     - None

hsdb>

--System.err:(147/8216)--

Attaching to process 23463, please wait...

javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54


javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54


Attaching to process 23731, please wait...

javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54


javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54


java.lang.NullPointerException

     at 
jdk.hotspot.agent/sun.jvm.hotspot.tools.StackTrace.run(StackTrace.java:83)


     at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor$26.doit(CommandProcessor.java:1073)


     at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1966)

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
On 2/7/18 3:23 PM,
  serguei.spit...@oracle.com wrote:


  
  Hi Chris,


On 2/7/18 15:06, Chris Plummer wrote:
  
  

Hi Paru,
  
  On 2/7/18 2:30 PM, Paru Somashekar wrote:


  Thanks for the review Chris, comments inline..
  On 2/7/18, 1:25 PM, Chris Plummer wrote:
  
Hi Paru,
  
  Thanks for writing this test. It will make figuring out JDK-8187143 a lot easier.
  
  Overall the test looks good. My main concern is the lack
  of comments. It makes it hard for the reader to understand
  the flow of the test and to understand some of the less
  obvious actions being performed. That is especially true
  for this test, which is doing some really bizarre stuff.
  Some of this you cover in our RFR summary below, but that
  info really needs to be in the test itself, along with
  additional comments. For example, what does
  pauseAtDebugger() do? It looks to me like it sets a
  breakpoint on the _javascript_ debugger that has a class
  name that ends with ScriptRuntime and the method name is
  DEBUGGER(). But I only figured that out after staring at
  the code for a while, and recalling a conversation we had
  a few weeks ago. It's also not described why this is being
  done.

  
  I shall add more comments into the test code to make it easier
  to understand. However while I understand what is being done (
  e.g. adding breakpoint on Nashorn's internal DEBUGGER method)
  - unfortunately I too am not sure "why" it is being done. I do
  not have insight on what the author ( bug reporter) was trying
  to do.. 

That's ok. They "why" is because this is a test case
demonstrating a failure a user ran into. You might want to
mention that also, although the @bug reference might enough.
  
  
  Agreed as this is my understanding too.
  
  
  

  
 
  Here's another example:
  
   126 while (!vmDisconnected) {
   127 try {
   128 Thread.sleep(100);
   129 } catch (InterruptedException ee) {
   130 }
   131 }
  
  I seem to also recall us discussing the need for this, but
  can no longer recall the reason

  
  The above loop is there to make the debugger continue to run
  until it receives a VMdisconnect event either because the
  Debuggee crashed / got exception / finished. 
  I shall add a comment for this as well. 
  
 
  Another example is findScriptFrame(). What is the
  significance of the frame whose class name starts with
  jdk.nashorn.internal.scripts.Script$? I think I understand
  (it's the generated java method for the _javascript_ you
  setup in ScriptDebuggee.doit()), but I can only figure
  this out based on earlier conversations we had and your
  RFR comments below. I'd expect the uninformed reader to
  spend a long time coming the same conclusion.

  
  Again, I am not clear on the significance of popping frames
  until this method which is a generated java method for
  _javascript_ in the debuggee. I could consult with the author
  and add those comments as well. 

This is just to recreate the situation the customer saw when
running into the bug. We don't need to know the details of why
they were doing what they did, only that it resulted in a bug
being exposed. I'm mostly asking that you add comments that
explain what the test is doing, but not worry so much about
explaining the underlying reasons why the test was written in
the first place (although that might be useful as part of an
overall test summary comment at the top).
  
  
  Right.
  Thank you for the suggestion!
  I did not pay attention to it when pre-reviewed.
  
  

  
 
  The following are just a few minor things I noticed:
  
  Copyright should only have 2018.
  
    57 } catch (Exception npe) {
  
  Probably best to call it "ex" instead of "npe".
  
    85 NashornPopFrameTest bbcT = new
  NashornPopFrameTest(args);
  
  It's unclear to

Re: RFR: 8041626: [Event Request] Shutdown reason

2018-02-07 Thread David Holmes


Hi Robin,

Adding in hotspot-runtime-dev as all the hotspot changes belong to runtime.

I had an initial look through this.

To be honest I don't like it. We seem to have to record little bits of 
information all over the place just so they can be reported by the 
shutdown event. It seems untidy. :(


Can you rename _starting_thread to _main_thread please. The use of 
"starting" in thread.hpp/cpp is really a naming anomaly. The main thread 
is the thread that loads the JVM. And that would be consistent with 
set_exception_in_main_thread.


Though why do we care if the main thread exited with an unhandled 
exception? And if we do care, why do we only care when the shutdown 
reason is ""No remaining non-daemon Java threads"?


I don't like the need to add os::get_abort_exit_code. Do we really need 
it? What useful information does it convey?


It is unfortunate that you need to add beforeHalt to deal with the event 
mechanism itself being turned off (?) by a shutdown hook. That would 
seem to potentially lose a lot of event information given hooks can run 
in arbitrary order and execute arbitrary Java code. And essentially you 
end up recording the initial reason shutdown started, though potentially 
it could end up terminating for a different reason.


Let's see what others think ...

Thanks,
David

On 8/02/2018 1:18 AM, Robin Westberg wrote:

Hi all,

Please review the following change that adds an event-based tracing 
event that is generated when the VM shuts down. The intent is to capture 
shutdowns that occur after the VM has been properly initialized (as 
initialization problems would most likely mean that the tracing 
framework hasn’t been properly started either).


Issue: https://bugs.openjdk.java.net/browse/JDK-8041626
Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/
Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2

Best regards,
Robin

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143

2018-02-07 Thread Paru Somashekar


Hi Chris, Serguei,

On 2/7/18, 4:56 PM, serguei.spit...@oracle.com wrote:

On 2/7/18 16:47, Chris Plummer wrote:
On 2/7/18 3:23 PM, serguei.spit...@oracle.com 
 wrote:

Hi Chris,


On 2/7/18 15:06, Chris Plummer wrote:

Hi Paru,

On 2/7/18 2:30 PM, Paru Somashekar wrote:

Thanks for the review Chris, comments inline..
On 2/7/18, 1:25 PM, Chris Plummer wrote:

Hi Paru,

Thanks for writing this test. It will make figuring out 
JDK-8187143  a 
lot easier.


Overall the test looks good. My main concern is the lack of 
comments. It makes it hard for the reader to understand the flow 
of the test and to understand some of the less obvious actions 
being performed. That is especially true for this test, which is 
doing some really bizarre stuff. Some of this you cover in our 
RFR summary below, but that info really needs to be in the test 
itself, along with additional comments. For example, what does 
pauseAtDebugger() do? It looks to me like it sets a breakpoint on 
the java script debugger that has a class name that ends with 
ScriptRuntime and the method name is DEBUGGER(). But I only 
figured that out after staring at the code for a while, and 
recalling a conversation we had a few weeks ago. It's also not 
described why this is being done.
I shall add more comments into the test code to make it easier to 
understand. However while I understand what is being done ( e.g. 
adding breakpoint on Nashorn's internal DEBUGGER method) - 
unfortunately I too am not sure "why" it is being done. I do not 
have insight on what the author ( bug reporter) was trying to do..
That's ok. They "why" is because this is a test case demonstrating 
a failure a user ran into. You might want to mention that also, 
although the @bug reference might enough.


Agreed as this is my understanding too.




Here's another example:

 126 while (!vmDisconnected) {
 127 try {
 128 Thread.sleep(100);
 129 } catch (InterruptedException ee) {
 130 }
 131 }

I seem to also recall us discussing the need for this, but can no 
longer recall the reason
The above loop is there to make the debugger continue to run until 
it receives a VMdisconnect event either because the Debuggee 
crashed / got exception / finished.

I shall add a comment for this as well.


Another example is findScriptFrame(). What is the significance of 
the frame whose class name starts with 
jdk.nashorn.internal.scripts.Script$? I think I understand (it's 
the generated java method for the javascript you setup in 
ScriptDebuggee.doit()), but I can only figure this out based on 
earlier conversations we had and your RFR comments below. I'd 
expect the uninformed reader to spend a long time coming the same 
conclusion.
Again, I am not clear on the significance of popping frames until 
this method which is a generated java method for javascript in the 
debuggee. I could consult with the author and add those comments 
as well.
This is just to recreate the situation the customer saw when 
running into the bug. We don't need to know the details of why they 
were doing what they did, only that it resulted in a bug being 
exposed. I'm mostly asking that you add comments that explain what 
the test is doing, but not worry so much about explaining the 
underlying reasons why the test was written in the first place 
(although that might be useful as part of an overall test summary 
comment at the top).


Right.
Thank you for the suggestion!
I did not pay attention to it when pre-reviewed.



The following are just a few minor things I noticed:

Copyright should only have 2018.

  57 } catch (Exception npe) {

Probably best to call it "ex" instead of "npe".

  85 NashornPopFrameTest bbcT = new 
NashornPopFrameTest(args);


It's unclear to me where the name "bbcT" comes from.

I shall change that to npft or something like that.


 134 if (failReason != null) {
 135 failure(failReason);
 136 }

You have two classes that declare "String failReason" which makes 
it a bit confusing to track which one the reader is dealing with. 
Also, the NashornPopFrameTest version is initialized to non-null, 
so doesn't that make the test always fail when the above code is 
executed?
Even though failReason is initialized, it is reset if the expected 
breakpoint is reached. And when the breakpoint is reached, it 
checks the Debuggee version of the field, if it is non null, then 
this field is set to the non null value - else it is set to null.

I shall add some comments to make it less confusing.

So in the above check failReason has a double meaning (maybe even 
triple). It could be set to its original value, which means the 
breakpoint was never reached, or if the breakpoint is reached it is 
set to ScriptDebuggee.failReason, which basically represents the 
result of having called engine.eval(script). I think it

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
On 2/7/18 16:47, Chris Plummer wrote:


  
  On 2/7/18 3:23 PM, serguei.spit...@oracle.com wrote:
  
  
Hi Chris,
  
  
  On 2/7/18 15:06, Chris Plummer wrote:


  Hi Paru,

On 2/7/18 2:30 PM, Paru Somashekar wrote:
  
  
Thanks for the review Chris, comments inline..
On 2/7/18, 1:25 PM, Chris Plummer wrote:

  Hi Paru,

Thanks for writing this test. It will make figuring out
JDK-8187143 a lot easier.

Overall the test looks good. My main concern is the lack
of comments. It makes it hard for the reader to
understand the flow of the test and to understand some
of the less obvious actions being performed. That is
especially true for this test, which is doing some
really bizarre stuff. Some of this you cover in our RFR
summary below, but that info really needs to be in the
test itself, along with additional comments. For
example, what does pauseAtDebugger() do? It looks to me
like it sets a breakpoint on the _javascript_ debugger
that has a class name that ends with ScriptRuntime and
the method name is DEBUGGER(). But I only figured that
out after staring at the code for a while, and recalling
a conversation we had a few weeks ago. It's also not
described why this is being done.
  

I shall add more comments into the test code to make it
easier to understand. However while I understand what is
being done ( e.g. adding breakpoint on Nashorn's internal
DEBUGGER method) - unfortunately I too am not sure "why" it
is being done. I do not have insight on what the author (
bug reporter) was trying to do.. 
  
  That's ok. They "why" is because this is a test case
  demonstrating a failure a user ran into. You might want to
  mention that also, although the @bug reference might enough.


Agreed as this is my understanding too.



  

   
Here's another example:

 126 while (!vmDisconnected) {
 127 try {
 128 Thread.sleep(100);
 129 } catch (InterruptedException ee) {
 130 }
 131 }

I seem to also recall us discussing the need for this,
but can no longer recall the reason
  

The above loop is there to make the debugger continue to run
until it receives a VMdisconnect event either because the
Debuggee crashed / got exception / finished. 
I shall add a comment for this as well. 

   
Another example is findScriptFrame(). What is the
significance of the frame whose class name starts with
jdk.nashorn.internal.scripts.Script$? I think I
understand (it's the generated java method for the
_javascript_ you setup in ScriptDebuggee.doit()), but I
can only figure this out based on earlier conversations
we had and your RFR comments below. I'd expect the
uninformed reader to spend a long time coming the same
conclusion.
  

Again, I am not clear on the significance of popping frames
until this method which is a generated java method for
_javascript_ in the debuggee. I could consult with the author
and add those comments as well. 
  
  This is just to recreate the situation the customer saw when
  running into the bug. We don't need to know the details of why
  they were doing what they did, only that it resulted in a bug
  being exposed. I'm mostly asking that you add comments that
  explain what the test is doing, but not worry so much about
  explaining the underlying reasons why the test was written in
  the first place (although that might be useful as part of an
  overall test summary comment at the top).


Right.
Thank you for the suggestion!
I did not pay attention to it when pre-reviewed.


  

   
The following are just a few minor things I noticed:

Copyright should only have 2018.

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

A fresh webrev rebased with latest jdk/jdk repos:
 http://cr.openjdk.java.net/~gadams/8080990/webrev.03/index.html

If there are no more comments, I'll check in locally with these reviewers
  clanger,chegar,erikj

and cut patches on Thurs AM.

On 2/5/18 2:15 PM, Gary Adams wrote:

One more to fix to cover the remaining test failures I was seeing.

Previously, using inet_addr, there was a single -1 return that needed 
to be checked.
Now that we're using inet_pton, there is a -1 and a 0 error return to 
be considered.

My preference would be to leave the dbgSysInetAddr name unchanged and 
have the low level
check for inet_pton errors to simply return the -1 that was previously 
checked.

This specific issue is about the deprecation of library functions on 
windows. I'd recommend not
upgrading the other platforms at this time until a complete update is 
done for ipv6 support.

I'll post a new webrev when I've retested using the updated inet_pton 
error checking.

...

Hi Gary,

>/Here's a revised webrev />//>/http://cr.openjdk.java.net/~gadams/8080990/webrev.01/index.html 
 />//>/Still testing ... />//>/Using shutdown() fixed problems reported by the />/java/nio/channelSocketChannel tests. /

The fix looks good. I would think we should rename dbgsysInetAddr to dbgsysPToN 
and use inet_pton also for the Unix/Linux platforms. It would be the better 
choice there as well, though we still only support IPv4 in libdt_socket.

>//>/I also noticed prior use of getaddrinfo for "localhost" was not calling 
/>/freeaddrinfo. /
Thanks for spotting/fixing that.

Best regards
Christoph

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
Hi Chris,
  
  
  On 2/7/18 15:06, Chris Plummer wrote:


  
  Hi Paru,

On 2/7/18 2:30 PM, Paru Somashekar wrote:
  
  
Thanks for the review Chris, comments inline..
On 2/7/18, 1:25 PM, Chris Plummer wrote:

  Hi Paru,

Thanks for writing this test. It will make figuring out JDK-8187143 a lot easier.

Overall the test looks good. My main concern is the lack of
comments. It makes it hard for the reader to understand the
flow of the test and to understand some of the less obvious
actions being performed. That is especially true for this
test, which is doing some really bizarre stuff. Some of this
you cover in our RFR summary below, but that info really
needs to be in the test itself, along with additional
comments. For example, what does pauseAtDebugger() do? It
looks to me like it sets a breakpoint on the _javascript_
debugger that has a class name that ends with ScriptRuntime
and the method name is DEBUGGER(). But I only figured that
out after staring at the code for a while, and recalling a
conversation we had a few weeks ago. It's also not described
why this is being done.
  

I shall add more comments into the test code to make it easier
to understand. However while I understand what is being done (
e.g. adding breakpoint on Nashorn's internal DEBUGGER method) -
unfortunately I too am not sure "why" it is being done. I do not
have insight on what the author ( bug reporter) was trying to
do.. 
  
  That's ok. They "why" is because this is a test case demonstrating
  a failure a user ran into. You might want to mention that also,
  although the @bug reference might enough.


Agreed as this is my understanding too.



  

   
Here's another example:

 126 while (!vmDisconnected) {
 127 try {
 128 Thread.sleep(100);
 129 } catch (InterruptedException ee) {
 130 }
 131 }

I seem to also recall us discussing the need for this, but
can no longer recall the reason
  

The above loop is there to make the debugger continue to run
until it receives a VMdisconnect event either because the
Debuggee crashed / got exception / finished. 
I shall add a comment for this as well. 

   
Another example is findScriptFrame(). What is the
significance of the frame whose class name starts with
jdk.nashorn.internal.scripts.Script$? I think I understand
(it's the generated java method for the _javascript_ you setup
in ScriptDebuggee.doit()), but I can only figure this out
based on earlier conversations we had and your RFR comments
below. I'd expect the uninformed reader to spend a long time
coming the same conclusion.
  

Again, I am not clear on the significance of popping frames
until this method which is a generated java method for
_javascript_ in the debuggee. I could consult with the author and
add those comments as well. 
  
  This is just to recreate the situation the customer saw when
  running into the bug. We don't need to know the details of why
  they were doing what they did, only that it resulted in a bug
  being exposed. I'm mostly asking that you add comments that
  explain what the test is doing, but not worry so much about
  explaining the underlying reasons why the test was written in the
  first place (although that might be useful as part of an overall
  test summary comment at the top).


Right.
Thank you for the suggestion!
I did not pay attention to it when pre-reviewed.


  

   
The following are just a few minor things I noticed:

Copyright should only have 2018.

  57 } catch (Exception npe) {

Probably best to call it "ex" instead of "npe".

  85 NashornPopFrameTest bbcT = new
NashornPopFrameTest(args);

It's unclear to me where the name "bbcT" comes from.
  

I shall change that to npft or something like that.

   
 134 if (failReason != null) {
 135 failure(failReason);
 136 }

You have two classes that declare

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
Hi Paru,
  
  On 2/7/18 2:30 PM, Paru Somashekar wrote:


  
  Thanks for the review Chris, comments inline..
  On 2/7/18, 1:25 PM, Chris Plummer wrote:
  

Hi Paru,
  
  Thanks for writing this test. It will make figuring out JDK-8187143
  a lot easier.
  
  Overall the test looks good. My main concern is the lack of
  comments. It makes it hard for the reader to understand the
  flow of the test and to understand some of the less obvious
  actions being performed. That is especially true for this
  test, which is doing some really bizarre stuff. Some of this
  you cover in our RFR summary below, but that info really needs
  to be in the test itself, along with additional comments. For
  example, what does pauseAtDebugger() do? It looks to me like
  it sets a breakpoint on the _javascript_ debugger that has a
  class name that ends with ScriptRuntime and the method name is
  DEBUGGER(). But I only figured that out after staring at the
  code for a while, and recalling a conversation we had a few
  weeks ago. It's also not described why this is being done.

  
  I shall add more comments into the test code to make it easier to
  understand. However while I understand what is being done ( e.g.
  adding breakpoint on Nashorn's internal DEBUGGER method) -
  unfortunately I too am not sure "why" it is being done. I do not
  have insight on what the author ( bug reporter) was trying to do..
  

That's ok. They "why" is because this is a test case demonstrating a
failure a user ran into. You might want to mention that also,
although the @bug reference might enough.

  
 
  Here's another example:
  
   126 while (!vmDisconnected) {
   127 try {
   128 Thread.sleep(100);
   129 } catch (InterruptedException ee) {
   130 }
   131 }
  
  I seem to also recall us discussing the need for this, but can
  no longer recall the reason

  
  The above loop is there to make the debugger continue to run until
  it receives a VMdisconnect event either because the Debuggee
  crashed / got exception / finished. 
  I shall add a comment for this as well. 
  
 
  Another example is findScriptFrame(). What is the significance
  of the frame whose class name starts with
  jdk.nashorn.internal.scripts.Script$? I think I understand
  (it's the generated java method for the _javascript_ you setup
  in ScriptDebuggee.doit()), but I can only figure this out
  based on earlier conversations we had and your RFR comments
  below. I'd expect the uninformed reader to spend a long time
  coming the same conclusion.

  
  Again, I am not clear on the significance of popping frames until
  this method which is a generated java method for _javascript_ in the
  debuggee. I could consult with the author and add those comments
  as well. 

This is just to recreate the situation the customer saw when running
into the bug. We don't need to know the details of why they were
doing what they did, only that it resulted in a bug being exposed.
I'm mostly asking that you add comments that explain what the test
is doing, but not worry so much about explaining the underlying
reasons why the test was written in the first place (although that
might be useful as part of an overall test summary comment at the
top).

  
 
  The following are just a few minor things I noticed:
  
  Copyright should only have 2018.
  
    57 } catch (Exception npe) {
  
  Probably best to call it "ex" instead of "npe".
  
    85 NashornPopFrameTest bbcT = new
  NashornPopFrameTest(args);
  
  It's unclear to me where the name "bbcT" comes from.

  
  I shall change that to npft or something like that.
  
 
   134 if (failReason != null) {
   135 failure(failReason);
   136 }
  
  You have two classes that declare "String failReason" which
  makes it a bit confusing to track which one the reader is
  dealing with. Also, the NashornPopFrameTest version is
  initialized to non-null, so doesn't that make the test always
  fail when the above code is executed?

  
  Even though failReason is initialized, it is reset if the expected
  breakpoint is reached. And when the breakpoint is reached, it
  checks the Debuggee version of the

Re: Issues debugging java 9 from jdk 8


  
  
Hi Egor,
  
  [adding jdk8u-dev, which is where 8u backports are discussed]
  
  I think major.minor changed from 1.8 to 9.0, although I haven't
  found the code to confirm that yet. I'm assuming this because of
  the following code:
  
      public boolean canGetModuleInfo() {
      validateVM();
      return versionInfo().jdwpMajor >= 9;
      }
  
  Given that, your changes look correct. I'm not an 8u reviewer.
  You'll need to get the official ok from someone on the 8u list.
  
  thanks,
  
  Chris
  
  On 2/5/18 7:44 AM, Egor Ushakov wrote:


  
  Hi all,
  in IDEA we faced an issue that when debugging java 9 process
  memory view does not work:
  https://youtrack.jetbrains.com/issue/JRE-641
  
  It seems that there's a bug in how
  VirtualMachineImpl.canGetInstanceInfo checks vm version (it does
  not pass jdk 9 where minor is 0):
  if (versionInfo().jdwpMajor < 1 ||
   versionInfo().jdwpMinor < 6) {
   return false;
  }
  
I've found this fixed in jdk 9 inside the fix:
  http://hg.openjdk.java.net/jdk9/hs/rev/f900d5afd9c8
  8142968: Module System implementation Summary: Initial integration
  of JEP 200, JEP 260, JEP 261, and JEP 282 alanb 17-Mar-16 22:04 
  
  We've applied the part of the fix into our jdk 8 fork:
  https://github.com/JetBrains/jdk8u_jdk/commit/6424e2786e8adc4e012e0b7bd0cfc78ba1ab34dd
  
  It seems reasonable to backport at least this part into openjdk 8?
  What do you think?
  Maybe some other parts deserve backporting as well.
  I've attached the patch just in case.
  -- 
Egor Ushakov
Software Developer
JetBrains
http://www.jetbrains.com
The Drive to Develop

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()


On 2/7/18 12:55 PM, gary.ad...@oracle.com wrote:

On 2/7/18 12:31 PM, Chris Hegarty wrote:

Gary,


http://cr.openjdk.java.net/%7Egadams/8080990/webrev.02/

I think the replacement of WSASendDisconnect with
shutdown(SD_SEND) should be fine. I do note that there
is another usage of WSASendDisconnect in
java.base/windows/native/libnet/net_util_md.c.

Thanks for the reference.
I'm not sure how that one slipped by unnoticed.

I found in make/lib/NetworkingLibraries.gmk the BUILD_LIBNET
target includes a number of specific disabled warnings. In particular,
4996 which suppresses all deprecated function warnings.

I think I'd like to get the current changes pushed to remove the specific
WINSOCK_DEPRECATED_NO_WARNINGS and then file another bug to
come back and look at selective makefiles that use a blanket 4996
suppression of deprecated warnings.



[ Maybe you want to separate out the changes in java.base
( NIO and NET ) from the serviceability changes? Up to
you. ]

Actually, this change  is about removing the compilation flag globally
in flags.m4 and any sources that need to be updated
to replace the deprecated functions.


Curious about the specific hints you have chosen to use.
In other areas we have the following:
   hints.ai_flags = AI_CANONNAME;
   hints.ai_family = AF_INET;

[ Not saying that what you have is incorrect, just questioning
  if you need to specify the socket type and protocol ]

One of the benefits of getaddrinfo is that it can return a list of
addresses from the name service. In the places we were using
gethostbyname we were looking for just one ipv4 address.
By adding additional constraints on the supplied hints, it can help
reduce the list of returned addresses.



-Chris.

Re: RFR (S): 8195115: G1 Old Gen MemoryPool CollectionUsage.used values don't reflect mixed GC results

2018-02-07 Thread Hohensee, Paul

I’ve filed https://bugs.openjdk.java.net/browse/JDK-8196989 : Revamp G1 JMX 
MemoryPool and GarbageCollector MXBean definitions and the corresponding CSR 
https://bugs.openjdk.java.net/browse/JDK-8196991.

Would you please comment on the CSR, and on the original CSR 
https://bugs.openjdk.java.net/browse/JDK-8196719?

Thanks,

Paul

On 2/2/18, 1:20 PM, "hotspot-gc-dev on behalf of Hohensee, Paul" 

wrote:

And, can a get a reviewer or reviewers for the CSR?

Thanks,

Paul

On 2/2/18, 1:14 PM, "Hohensee, Paul"  wrote:

+hotspot-gc-use.

I’ve filed a CSR for the current patch, see 
https://bugs.openjdk.java.net/browse/JDK-8196719. Here’s the argument in favor.

It’s possible that there are JDK8 users that rely on current G1 old gen 
CollectionUsage behavior, but imo it’s unlikely because it’s of little use. 
Perhaps Kirk and others with operational experience can weigh in.

Let’s think about use cases. G1 full GC’s happen rarely and only under 
severe pressure, so when they do external reaction is pretty much limited to 
reducing load so the JVM can get back to a usable steady state, or just 
restarting the JVM. Old gen CollectionUsage is zero until a full GC occurs, 
after which its value includes both long-lived objects and any transient data 
that was in eden and the survivor space. That value doesn’t tell you anything 
about long term old gen occupancy or survivor size because it lumps them all 
together. So, it isn’t a useful metric, nor will it be after any subsequent 
full GCs. The only information it provides is on the first zero to non-zero 
transition, which just tells you that the JVM is/was in trouble. Further, the 
effect of the runup to a full GC is SLA violations, which are noticed before 
the full GC happens, so detecting the first full GC is confirmation, not 
prediction.

Conclusion: G1 old gen CollectionUsage is unlikely to be in use in its 
current form, so changing its definition to something usable is low risk.

“G1 Old Space” is fine, as is “G1 Archive Space”. Are you ok with the 
G1 archive space overlapping the G1 old space? Should we add an archive space 
to the other collectors? If so, how would it be defined and would having it 
overlap with the old generation as a live prefix be ok?

"G1 Young Generation" is the currently young+mixed collector.

You’re right, if one is iterating over all collectors, there will be 
redundancy if we keep the old ones. I’m usually leery of introducing a flag 
such as UseG1LegacyMXBeans (I changed the name, since all the interfaces are 
MXBeans, hope that’s ok) which must be either indefinitely maintained, or go 
through a deprecation cycle. I don’t see a way out of the ‘iterate over all 
collectors’ problem without it though.

Paul

On 1/31/18, 3:42 AM, "Erik Helin"  wrote:

On 01/31/2018 02:30 AM, Hohensee, Paul wrote:
> It’s true that my patch doesn’t completely solve the larger 
problem, but it fixes the most immediately important part of it, particularly 
for JDK8 where current expected behavior is entrenched.

Yes, your patch fixes part of the problem, but as I said, can 
potentially lead to more confusion. I'm not sure that doing this 
behavioral change for a public API in an JDK 8 update release is 
the 
right thing. There are likely users that rely on the memory pool 
"G1 Old 
Gen" only being updated by a full collection (even though that 
behavior 
is not correct), those uses will encounter a new behavior in an 
update 
release with your patch.

The good thing is that we have very experienced engineers 
participating 
in the CSR process that have much more experience than I have in 
evaluating the impact of behavioral changes such as this one. Would 
you 
please file a CSR request for your patch to get their opinion?

See https://wiki.openjdk.java.net/display/csr/Main for more details 
about CSR.

On 01/31/2018 02:30 AM, Hohensee, Paul wrote:
If we’re going to fix the larger problem, imo we should file 
another 
bug/rfe to do it. I’d be happy to fix that one too, once we have a 
spec.
> 
> What do you think of my suggestions? To summarize:
> 
> - Keep the existing memory pools and add humongous and archive 
pools.
> - Make the archive pool part of the old gen, and generalize it to 
all collectors.
> - Split humongous regions off from the old gen pool into their 
own pool. The old gen and

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143

2018-02-07 Thread Paru Somashekar


Thanks for the review Serguei.

thanks,
Paru.

On 2/7/18, 12:25 PM, serguei.spit...@oracle.com wrote:

Hi Paru,

It looks good.
Thank you a lot for taking care about this!

Could we get at least one more review from the Serviceability team on 
this new test?


Thanks,
Serguei


On 2/2/18 09:35, Paru Somashekar wrote:

Hi,

Please review the fix for JDK-8193150.

The fix introduces a new jtreg test, NashornPopFrameTest. It is based 
on the original test from JDK-8187143 
 that was provided 
by the customer.


Bug : https://bugs.openjdk.java.net/browse/JDK-8193150 

Webrev : http://cr.openjdk.java.net/~psomashe/8193150/webrev/ 



Here is a brief description of what the test does :-

* The debuggee,  creates and uses a Nashorn engine to evaluate a 
simple script.


* The debugger  tries to set a breakpoint in Nashorn’s internal 
DEBUGGER method.
* When the breakpoint is reached, it looks for stack frame whose 
method's declaring type name starts with (nashorn dynamically 
generated classes) ”jdk.nashorn.internal.scripts.Script$”.
* It then pops stack frames using the ThreadReference.popFrames() 
call, up to and including the above stackframe.
* The execution of the debuggee application is resumed after the 
needed frames have been popped.


This test is included in the ProblemList as it fails under some 
circumstances (bug JDK-8187143) 
. Is always passes 
with the -Xint flag however always fails with -Xcomp. It fails 
intermittently with the -Xmixed (default).


thanks,
Paru.

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
Hi Paru,
  
  Thanks for writing this test. It will make figuring out JDK-8187143
  a lot easier.
  
  Overall the test looks good. My main concern is the lack of
  comments. It makes it hard for the reader to understand the flow
  of the test and to understand some of the less obvious actions
  being performed. That is especially true for this test, which is
  doing some really bizarre stuff. Some of this you cover in our RFR
  summary below, but that info really needs to be in the test
  itself, along with additional comments. For example, what does
  pauseAtDebugger() do? It looks to me like it sets a breakpoint on
  the _javascript_ debugger that has a class name that ends with
  ScriptRuntime and the method name is DEBUGGER(). But I only
  figured that out after staring at the code for a while, and
  recalling a conversation we had a few weeks ago. It's also not
  described why this is being done.
  
  Here's another example:
  
   126 while (!vmDisconnected) {
   127 try {
   128 Thread.sleep(100);
   129 } catch (InterruptedException ee) {
   130 }
   131 }
  
  I seem to also recall us discussing the need for this, but can no
  longer recall the reason
  
  Another example is findScriptFrame(). What is the significance of
  the frame whose class name starts with
  jdk.nashorn.internal.scripts.Script$? I think I understand (it's
  the generated java method for the _javascript_ you setup in
  ScriptDebuggee.doit()), but I can only figure this out based on
  earlier conversations we had and your RFR comments below. I'd
  expect the uninformed reader to spend a long time coming the same
  conclusion.
  
  The following are just a few minor things I noticed:
  
  Copyright should only have 2018.
  
    57 } catch (Exception npe) {
  
  Probably best to call it "ex" instead of "npe".
  
    85 NashornPopFrameTest bbcT = new
  NashornPopFrameTest(args);
  
  It's unclear to me where the name "bbcT" comes from.
  
   134 if (failReason != null) {
   135 failure(failReason);
   136 }
  
  You have two classes that declare "String failReason" which makes
  it a bit confusing to track which one the reader is dealing with.
  Also, the NashornPopFrameTest version is initialized to non-null,
  so doesn't that make the test always fail when the above code is
  executed?
  
  Is there a reason why ScriptDebuggee doesn't just put everything
  in main() and not have a doit() method?
  
  thanks,
  
  Chris
  
  On 2/7/18 12:25 PM, serguei.spit...@oracle.com wrote:


  
  Hi Paru,

It looks good.
Thank you a lot for taking care about this!

Could we get at least one more review from the Serviceability
team on this new test?

Thanks,
Serguei


On 2/2/18 09:35, Paru Somashekar wrote:
  
  

Hi, 

Please review the fix for JDK-8193150. 

The fix introduces a new jtreg test, NashornPopFrameTest. It is
based on the original test from JDK-8187143 that was provided by
the customer.

Bug : https://bugs.openjdk.java.net/browse/JDK-8193150

Webrev : http://cr.openjdk.java.net/~psomashe/8193150/webrev/

Here is a brief description of what the test does :-

* The debuggee,  creates and uses a Nashorn engine to evaluate a
simple script. 

* The debugger  tries to set a breakpoint in Nashorn’s internal
DEBUGGER method.
* When the breakpoint is reached, it looks for stack frame whose
method's declaring type name starts with (nashorn dynamically
generated classes) ”jdk.nashorn.internal.scripts.Script$”. 
* It then pops stack frames using the
ThreadReference.popFrames() call, up to and including the above
stackframe.
* The execution of the debuggee application is resumed after the
needed frames have been popped.

This test is included in the ProblemList as it fails under some
circumstances (bug JDK-8187143). Is always passes with
the -Xint flag however always fails with -Xcomp. It fails
intermittently with the -Xmixed (default). 

thanks,
Paru.

Re: RFR: 8041626: [Event Request] Shutdown reason

2018-02-07 Thread Robin Westberg

Hi Alan,

> On 7 Feb 2018, at 16:30, Alan Bateman  wrote:
> 
> On 07/02/2018 15:18, Robin Westberg wrote:
>> Hi all,
>> 
>> Please review the following change that adds an event-based tracing event 
>> that is generated when the VM shuts down. The intent is to capture shutdowns 
>> that occur after the VM has been properly initialized (as initialization 
>> problems would most likely mean that the tracing framework hasn’t been 
>> properly started either).
>> 
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8041626
>> Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/ 
>> 
>> Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2
>> 
> Can you elaborate a bit on why this isn't in JVM_Halt? Is this partially to 
> help with cases where the shutdown hooks or finalizers run at exit cause 
> issues?

Sure, the main problem I had with that approach is actually that the tracing 
framework shuts down from a shutdown hook, so events generated in JVM_Halt will 
be lost. And I couldn’t think of any good way to capture the exit code and 
proper stack trace from inside that shutdown hook, but I could perhaps explore 
that option a bit further.

Best regards,
Robin

Re: RFR: 8041626: [Event Request] Shutdown reason

2018-02-07 Thread Erik Gahlin


Hi Robin,

I can sponsor this.

I wonder if we should change the name of the event to "Shutdown" 
instead? It will give us flexibility to add other shutdown related 
fields in the future.


Could you change the label and field to "Status Code" and statusCode.  
Event fields should have headline-style capitalization and statusCode 
allows us to add other status related information in the future.


Thanks
Erik


Hi all,

Please review the following change that adds an event-based tracing 
event that is generated when the VM shuts down. The intent is to 
capture shutdowns that occur after the VM has been properly 
initialized (as initialization problems would most likely mean that 
the tracing framework hasn’t been properly started either).


Issue: https://bugs.openjdk.java.net/browse/JDK-8041626
Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/ 


Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2

Best regards,
Robin

Re: 8196977: add test fo JDK-8161605


Hi Alex,

Looks good.
As the test was already reviewed should the 'trivial fix' rule apply here?
One reviewer would be enough then.
Or just add reviewers from the 8161605 to this fix as well.

Thanks,
Serguei


On 2/7/18 10:47, Alex Menkov wrote:

Pushing fix for JDK-8161605, I forgot to add the test.
Please review the correction.

jira: https://bugs.openjdk.java.net/browse/JDK-8196977
webrev: http://cr.openjdk.java.net/~amenkov/forgotten_test/webrev/

--alex

Re: RFR 8193150: Create a jtreg version of the test from JDK-8187143


  
  
Hi Paru,
  
  It looks good.
  Thank you a lot for taking care about this!
  
  Could we get at least one more review from the Serviceability team
  on this new test?
  
  Thanks,
  Serguei
  
  
  On 2/2/18 09:35, Paru Somashekar wrote:


  
  Hi, 
  
  Please review the fix for JDK-8193150. 
  
  The fix introduces a new jtreg test, NashornPopFrameTest. It is
  based on the original test from JDK-8187143 that was provided by the
  customer.
  
  Bug : https://bugs.openjdk.java.net/browse/JDK-8193150
  
  Webrev : http://cr.openjdk.java.net/~psomashe/8193150/webrev/
  
  Here is a brief description of what the test does :-
  
  * The debuggee,  creates and uses a Nashorn engine to evaluate a
  simple script. 
  
  * The debugger  tries to set a breakpoint in Nashorn’s internal
  DEBUGGER method.
  * When the breakpoint is reached, it looks for stack frame whose
  method's declaring type name starts with (nashorn dynamically
  generated classes) ”jdk.nashorn.internal.scripts.Script$”. 
  * It then pops stack frames using the ThreadReference.popFrames()
  call, up to and including the above stackframe.
  * The execution of the debuggee application is resumed after the
  needed frames have been popped.
  
  This test is included in the ProblemList as it fails under some
  circumstances (bug JDK-8187143). Is always passes with
  the -Xint flag however always fails with -Xcomp. It fails
  intermittently with the -Xmixed (default). 
  
  thanks,
  Paru.

Re: RFR: JDK-8031445: Attach on windows can fail with java.io.IOException: All pipe instances are busy


Hi Gary,

I don't think you intended to include the ProblemList.txt changes in 
your webrev.


I think your changes address the "java.io.IOException: CreateNamedPipe 
failed" failures if a name collision is the cause. This failure mode was 
extremely rare (only 3 sightings), and if due to a collision, a single 
retry should suffice in making it not appear again in our lifetime. 
However, I don't think this addresses the "java.io.IOException: All pipe 
instances are busy" issue, which seems to the more common failures mode, 
although also very rare. Have you looked into its potential cause?


thanks,

Chris

On 2/7/18 8:51 AM, gary.ad...@oracle.com wrote:

The IOException that is observed when creating a new named pipe
when the pipe already exists and is in use, recommends to retry
the operation later. Since we are already using a random number
to generate a unique pipe name, it makes sense to simply
retry the operation with a new pipe name.

Here is a proposed fix. Testing in progress.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8031445
  Webrev: http://cr.openjdk.java.net/~gadams/8031445/

8196977: add test fo JDK-8161605

2018-02-07 Thread Alex Menkov


Pushing fix for JDK-8161605, I forgot to add the test.
Please review the correction.

jira: https://bugs.openjdk.java.net/browse/JDK-8196977
webrev: http://cr.openjdk.java.net/~amenkov/forgotten_test/webrev/

--alex

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()


On 2/7/18 12:48 PM, Alan Bateman wrote:

On 07/02/2018 17:31, Chris Hegarty wrote:

Gary,


http://cr.openjdk.java.net/%7Egadams/8080990/webrev.02/

I think the replacement of WSASendDisconnect with
shutdown(SD_SEND) should be fine. I do note that there
is another usage of WSASendDisconnect in
java.base/windows/native/libnet/net_util_md.c.

[ Maybe you want to separate out the changes in java.base
( NIO and NET ) from the serviceability changes? Up to
you. ]

Is NIO SocketDispatcher change needed in this patch? I would prefer if 
we could separate this was WSASendDisconnect was the semantics that we 
need.


-Alan

Yes, WSASendDisconnect is deprecated in vs2013.
As far as I know "shutdown(fd, SD_SEND)" prevents further outgoing writes
and there was no final payload to send.

I have not seen any failures in the java/nio tests.

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()


On 2/7/18 12:31 PM, Chris Hegarty wrote:

Gary,


http://cr.openjdk.java.net/%7Egadams/8080990/webrev.02/

I think the replacement of WSASendDisconnect with
shutdown(SD_SEND) should be fine. I do note that there
is another usage of WSASendDisconnect in
java.base/windows/native/libnet/net_util_md.c.

Thanks for the reference.
I'm not sure how that one slipped by unnoticed.


[ Maybe you want to separate out the changes in java.base
( NIO and NET ) from the serviceability changes? Up to
you. ]

Actually, this change  is about removing the compilation flag globally
in flags.m4 and any sources that need to be updated
to replace the deprecated functions.


Curious about the specific hints you have chosen to use.
In other areas we have the following:
   hints.ai_flags = AI_CANONNAME;
   hints.ai_family = AF_INET;

[ Not saying that what you have is incorrect, just questioning
  if you need to specify the socket type and protocol ]

One of the benefits of getaddrinfo is that it can return a list of
addresses from the name service. In the places we were using
gethostbyname we were looking for just one ipv4 address.
By adding additional constraints on the supplied hints, it can help
reduce the list of returned addresses.



-Chris.

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()


On 07/02/2018 17:31, Chris Hegarty wrote:

Gary,


http://cr.openjdk.java.net/%7Egadams/8080990/webrev.02/

I think the replacement of WSASendDisconnect with
shutdown(SD_SEND) should be fine. I do note that there
is another usage of WSASendDisconnect in
java.base/windows/native/libnet/net_util_md.c.

[ Maybe you want to separate out the changes in java.base
( NIO and NET ) from the serviceability changes? Up to
you. ]

Is NIO SocketDispatcher change needed in this patch? I would prefer if 
we could separate this was WSASendDisconnect was the semantics that we need.


-Alan

Re: JDK-8080990: libdt_socket/socket_md.c(202) : warning C4996: 'gethostbyname': Use getaddrinfo() or GetAddrInfoW()

2018-02-07 Thread Chris Hegarty

Gary,

> http://cr.openjdk.java.net/%7Egadams/8080990/webrev.02/

I think the replacement of WSASendDisconnect with
shutdown(SD_SEND) should be fine. I do note that there
is another usage of WSASendDisconnect in
java.base/windows/native/libnet/net_util_md.c.

[ Maybe you want to separate out the changes in java.base
( NIO and NET ) from the serviceability changes? Up to
you. ]

Curious about the specific hints you have chosen to use.
In other areas we have the following:
  hints.ai_flags = AI_CANONNAME;
  hints.ai_family = AF_INET;

[ Not saying that what you have is incorrect, just questioning
 if you need to specify the socket type and protocol ]

-Chris.

RFR: JDK-8031445: Attach on windows can fail with java.io.IOException: All pipe instances are busy


The IOException that is observed when creating a new named pipe
when the pipe already exists and is in use, recommends to retry
the operation later. Since we are already using a random number
to generate a unique pipe name, it makes sense to simply
retry the operation with a new pipe name.

Here is a proposed fix. Testing in progress.

  Issue: https://bugs.openjdk.java.net/browse/JDK-8031445
  Webrev: http://cr.openjdk.java.net/~gadams/8031445/

Issue with 8192897: NPE occurs on clhsdb jstack

2018-02-07 Thread stewartd . qdt

I am getting Null Pointer Exceptions with both ClhsdbJstack.java and 
ClhsdbFindPC.java. It appears the addition of testing with  '-Xcomp` causing a 
Method to come back as null during the stack walk. I have included the results 
of the Jstack test (the FindPC is similar) for the Xcomp section below. I have 
created bug JDK-8196969.  The 
full .jtr is attached there.  I am not familiar enough with the Java stack to 
know if having  a null Method pointer is valid or not.

Thanks,
Daniel

hsdb>
Command line: 
['/home/stewartd/openjdk/OpenJDK/hs/build/linux-aarch64-normal-server-release/images/jdk/bin/java'
 '-Xcomp' '-cp' 
'/home/stewartd/openjdk/OpenJDK/hs/JTwork/classes/serviceability/sa/ClhsdbJstack.d:/home/stewartd/openjdk/OpenJDK/hs/JTwork/classes/test/lib'
 'jdk.test.lib.apps.LingeredApp' '717a6b55-bcf8-405e-9d5a-afe1212f0fc6.lck' ]
Started LingeredApp (-Xcomp) with pid 23731
Starting clhsdb against 23731
Warning! JS Engine can't start, some commands will not be available.
hsdb> Deadlock Detection:

No deadlocks found.

"Common-Cleaner" #22 daemon prio=8 tid=0xa0528000 nid=0x5d2d in 
Object.wait() [0xfffe427de000]
   java.lang.Thread.State: TIMED_WAITING (on object monitor)
   JavaThread state: _thread_blocked
- java.lang.ref.ReferenceQueue.remove(long) @bci=59, line=151, 
pc=0x892af7c4, Method*=0xfffe49851a98 (Compiled frame; information 
may be imprecise)
- waiting to lock <0x000101c956f0> (a 
java.lang.ref.ReferenceQueue$Lock)
- jdk.internal.ref.CleanerImpl.run() @bci=65, line=148, pc=0x892ac8e4, 
Method*=0xfffe49ab23d8 (Compiled frame)
- java.lang.Thread.run() @bci=11, line=844, pc=0x892a9d30, 
Method*=0xfffe49738ee0 (Compiled frame)
- jdk.internal.misc.InnocuousThread.run() @bci=20, line=134, 
pc=0x892a9d30, Method*=0xfffe49aba318 (Compiled frame)

Locked ownable synchronizers:
- None

"Signal Dispatcher" #4 daemon prio=9 tid=0xa04a6800 nid=0x5d05 runnable 
[0x]
   java.lang.Thread.State: RUNNABLE
   JavaThread state: _thread_blocked

Locked ownable synchronizers:
- None

"Finalizer" #3 daemon prio=8 tid=0xa0494000 nid=0x5d04 in Object.wait() 
[0xfffe492ee000]
   java.lang.Thread.State: WAITING (on object monitor)
   JavaThread state: _thread_blocked
- java.lang.ref.ReferenceQueue.remove(long) @bci=59, line=151, 
pc=0x888e15a0, Method*=0xfffe49851a98 (Interpreted frame)
- waiting to lock <0x000101c09528> (a 
java.lang.ref.ReferenceQueue$Lock)
- java.lang.ref.ReferenceQueue.remove() @bci=2, line=172, 
pc=0x888e12c0, Method*=0xfffe49851b40 (Interpreted frame)
- java.lang.ref.Finalizer$FinalizerThread.run() @bci=37, line=216, 
pc=0x888e12c0, Method*=0xfffe49856cb0 (Interpreted frame)

Locked ownable synchronizers:
- None

"Reference Handler" #2 daemon prio=10 tid=0xa0492000 nid=0x5d03 waiting 
on condition [0xfffe494ee000]
   java.lang.Thread.State: RUNNABLE
   JavaThread state: _thread_blocked
- java.lang.ref.Reference.processPendingReferences() @bci=0, line=166, 
pc=0x888e15a0, Method*=0xfffe49733100 (Interpreted frame)
- java.lang.ref.Reference.access$000() @bci=0, line=44, pc=0x888e15a0, 
Method*=0xfffe49733788 (Interpreted frame)
- java.lang.ref.Reference$ReferenceHandler.run() @bci=0, line=138, 
pc=0x888e15a0, Method*=0xfffe4984ffa0 (Interpreted frame)

Locked ownable synchronizers:
- None

"main" #1 prio=5 tid=0xa001 nid=0x5cb5 waiting on condition 
[0xa5d2e000]
   java.lang.Thread.State: TIMED_WAITING (sleeping)
   JavaThread state: _thread_blocked
Error occurred during stack walking:

Locked ownable synchronizers:
- None

hsdb>
--System.err:(147/8216)--
Attaching to process 23463, please wait...
javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54
javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54
Attaching to process 23731, please wait...
javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54
javax.script.ScriptException: TypeError: sapkg.runtime.VM.getVM is not a 
function in sa.js at line number 54
java.lang.NullPointerException
at 
jdk.hotspot.agent/sun.jvm.hotspot.tools.StackTrace.run(StackTrace.java:83)
at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor$26.doit(CommandProcessor.java:1073)
at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1966)
at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.executeCommand(CommandProcessor.java:1936)
at 
jdk.hotspot.agent/sun.jvm.hotspot.CommandProcessor.run(CommandProcessor.java:1816)
at

Re: RFR: 8041626: [Event Request] Shutdown reason