Re: Remove unused import proposal: in JdpController.java

[David Holmes]

Hi Andrew,

I've filed:

https://bugs.openjdk.java.net/browse/JDK-8198539

to clean up your leftover import from JDK-8183123 :) (which was fixed in 
10 not 9).


If someone from serviceability doesn't pick this up I may be able to.

David

On 22/02/2018 1:29 AM, Andrew Leonard wrote:

Hi,
I would like to find a sponsor please for this simple tidy up of 
JdpController.java ?
It contains an unused "import sun.management.VMManagement;" which is for 
a sun specific java.management class which this class used to use prior 
to jdk9.

Thanks
Andrew

diff --git 
a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 
b/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 

--- 
a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 

+++ 
b/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 


@@ -34,7 +34,6 @@
  import java.lang.reflect.Field;
  import java.lang.reflect.Method;
  import java.lang.UnsupportedOperationException;
-import sun.management.VMManagement;

  /**
   * JdpController is responsible to create and manage a broadcast loop.





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


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598.

Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: Thread Native ID Access

[Thomas Stüfe]

Hi Jeremy,

On Wed, Feb 21, 2018 at 11:40 PM, Jeremy Manson 
wrote:

> Hey folks,
>
> I mentioned earlier in the thread about the ThreadInfo.from() bug that I
> found this because I was looking at fixing JDK-8154176, which proposes
> introducing native thread ids to Thread and ThreadInfo.
>
> https://bugs.openjdk.java.net/browse/JDK-8154176
>
> I have a prototype for it.  I have a couple of questions, though:
>
> 0) Does anyone object to this being done or my doing it?  I see that it
> already has an owner.
>
> 1) Should the ID be a long?  The Hotspot thread dump prints it out as
> 0x%x, which is an unsigned hexadecimal integer.  The type hiding behind it
> is platform-dependent, though: a pid_t on Linux, an unsigned long on
> Windows, a thread_t on Solaris.  I could make it a String instead...
>
>
Just to chime in on this, I would be fine with it. We (SAP) maintain AIX,
linux ppc and s390. Internally we also have a (closed) port for hpux and
AS/400. On all these platforms we have thread ids which can be expressed as
numericals. Neither do I know a contemporary platform which does not have
that, does anyone? And even if they exist, seeing how reluctant we are to
allow new platforms (e.g. see the discussions about letting in the BSD
people into the main line), I cannot see these platforms being porting
targets soon?

But please make it 64bit. At least AIX has 64bit thread ids. That also
gives porters a back door which want or have to hide the real thread id by
just returning e.g.  (probably not that, but something thread
specific which can be represented by a pointer on 64bit platforms).

Kind Regards, Thomas


Jeremy
>

Re: Thread Native ID Access

[David Holmes]

Hi Jeremy,

I think "thread id" is too platform specific to be exposed in a core 
Java API like Thread - even as a non-descript String.


Maybe through a platform specific MXBean API?

For things like  /proc//task/ is there a way to say  in a 
way that means "current thread"? That may be a partial solution - or 
even whole if you could then read back the actual id?


David


On 22/02/2018 8:40 AM, Jeremy Manson wrote:

Hey folks,

I mentioned earlier in the thread about the ThreadInfo.from() bug that I 
found this because I was looking at fixing JDK-8154176, which proposes 
introducing native thread ids to Thread and ThreadInfo.


https://bugs.openjdk.java.net/browse/JDK-8154176

I have a prototype for it.  I have a couple of questions, though:

0) Does anyone object to this being done or my doing it?  I see that it 
already has an owner.


1) Should the ID be a long?  The Hotspot thread dump prints it out as 
0x%x, which is an unsigned hexadecimal integer.  The type hiding behind 
it is platform-dependent, though: a pid_t on Linux, an unsigned long on 
Windows, a thread_t on Solaris.  I could make it a String instead...


Jeremy

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

[Harsha Wardhana B]

On Wednesday 21 February 2018 09:51 PM, mandy chung wrote:



On 2/20/18 9:55 PM, Harsha Wardhana B wrote:


We cannot get rid of specifying options via -D. We have plenty of -D 
flags but very few have short-hand alternative via 
--start-management-agent. If management properties are specified by 
--start-management-agent, the options specified by -D are anyway 
overwritten if specified.


I have to see the specification of this feature and give further 
feedback.   If the new option does not accept all management 
properties, what is the benefit of this new option?
The new options makes it easy to start and configure default agent. As 
of now, users needs to be aware of -D flags which are long and 
cumbersome. Hence only most frequently used -D flags are given the 
short-hand alternative to --start-management-agent.


Harsha


Mandy

Re: Thread Native ID Access

[mandy chung]

JDK 5 did very minimal for monitoring of native OS resources due to its 
scope and also its platform-dependent nature. I don't have a good advice 
how and where to expose them via Java SE API without further 
investigation. A couple of things come to mind: - Should native threads 
be exposed in Process API? - jdk.management module defines JDK extension 
to java.lang.management API that might be a good place to list VM 
threads. - other than VM threads and Java to native OS threads mapping, 
any other items on your list for monitoring? Mandy [1] 
http://download.java.net/java/jdk10/docs/api/jdk.management-summary.html


On 2/21/18 3:46 PM, Jeremy Manson wrote:
Agreed that it is platform-dependent.  That said, so is PID, and you  > can now get that from java.lang.Process. And also, there is very > 
little available in SIGQUIT thread dumps that you can't get > 
programmatically, and this is one of the big ticket things (we also > 
have a patch to export the list of VM threads; maybe that would be > 
useful to expose somewhere). > > Although the nid is useful for 
troubleshooting, it's more often used > for ongoing VM monitoring, 
because you often want to map from Java > Thread to some native 
resource, and the only way to do it is via > native thread id. For 
examples of this sort of thing, on Linux, look > in 
/proc//task/. There's a lot of useful information in > there, 
but how to map it to Java threads is non-obvious. > > At Google, we 
added an interface to export the information, but I > would rather 
relieve ourselves of the (relatively minor) technical > debt and 
contribute to the community. > > I'm generally a little skeptical that 
adding to a JDK-specific > diagnostic tool is the right solution. At 
Google, at least, it can > be very, very difficult to apply those tools 
at scale / integrate > them into existing monitoring tooling. When the 
JDK has > functionality that is only available via one of these 
commands, we > usually end up having to figure out what the command is 
doing and > reimplement it. So, for example, we ended up with our own 
parser for > hsperfdata. We end up relying on undocumented interfaces 
that can go > away at the next JDK revision, and that's not good. > > I 
have no problem with some other, documented interface for it that > 
doesn't go through Thread/ThreadInfo, but we (at least) will end up > 
needing something that isn't tied to a particular tool. > > As far as 
the interaction of a feature like this with Fibers (or > other alternate 
threading approaches, like Green threads): I wouldn't > be worried about 
that. A fiber executes on an os thread. The only > thing that might 
happen is that the os thread might change over time, > or that multiple 
fibers might multiplex on the same os thread. If > you are worried about 
the value changing, all you have to do is not > to cache it in the 
Thread object. You wouldn't want to do that > anyway, because it would 
be a waste of space for something that's > only needed occasionally. > > 
Is there another way of doing the right thing here? > > Jeremy > > > > 
On Wed, Feb 21, 2018 at 3:04 PM, mandy chung  
> wrote: > > I'm not comfortable for 
ThreadInfo to expose the implementation > details. What should we 
specify w.r.t. Java Thread mapping to native > thread which is platform 
dependent. Also how does it relate to the > future fibers [1]? > > 
Another alternative is for JDK specific diagnostic tools to do that > 
mapping for you for example jcmd, rather than exposing it in the API. > 
I assume is that this is more about troubleshooting than on-going VM > 
monitoring. > > Mandy [1] http://openjdk.java.net/projects/loom/ > 
 > > > On 2/21/18 2:40 PM, 
Jeremy Manson wrote: >> Hey folks, >> >> I mentioned earlier in the 
thread about the ThreadInfo.from() bug >> that I found this because I 
was looking at fixing JDK-8154176, >> which proposes introducing native 
thread ids to Thread and >> ThreadInfo. >> >> 
https://bugs.openjdk.java.net/browse/JDK-8154176 >> 
 >> >> I have a 
prototype for it. I have a couple of questions, though: >> >> 0) Does 
anyone object to this being done or my doing it? I see >> that it 
already has an owner. >> >> 1) Should the ID be a long? The Hotspot 
thread dump prints it out >> as 0x%x, which is an unsigned hexadecimal 
integer. The type hiding >> behind it is platform-dependent, though: a 
pid_t on Linux, an >> unsigned long on Windows, a thread_t on Solaris. I 
could make it a >> String instead... >> >> Jeremy > >

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

On 2/21/18 16:59, David Holmes wrote:

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
 // deleted methods must be private
 return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the 
above to:

   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".


There are multiple negations above, so this needs to be interpreted in a 
right context:

  Return error if deleted method is !PRIVATE or (!FINAL and !STATIC)

After inversion:
  Allow to delete if method is PRIVATE and (FINAL or STATIC)

I feel myself stupid when reading this code. :)

Thanks,
Serguei


David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be 
deleted”)


I may have read the code incorrectly - but I would expect to hit 
this assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it handles final 
fields.

option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit 
the assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> 
wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI 
documentation to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to 
fix it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation 
to relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also 
unclear whether the add/delete is restricted to final/static 
methods or any private method? I can see that the intent was to 
only allow something that would not perturb the vtable for 
existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in 

Re: JVMTI retransformation and addition of private methods

[David Holmes]

Correction ...

On 22/02/2018 10:59 AM, David Holmes wrote:

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
 // deleted methods must be private
 return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the 
above to:

   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".


Nope I read it wrong - Serguei is right.

Which is somewhat strange as the underlying problem being addressed 
seemed to require retransformation of Object.wait and Thread.sleep - the 
former is a public final instance method; the latter a public static 
method. Neither are private ... ???


David


David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit the 
assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> 
wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to 
fix it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late 

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

  
  
Karen,
  
  
  On 2/21/18 16:46, Karen Kinnear wrote:


  
  Sergeui,
  
  
  You were right - I read the sources incorrectly.


This code is easy to misunderstand - I read it incorrectly multiple
times. :)
This parts causes most of confusion:
  (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0


   Would still help to understand both
  the motivation and the reason to not add to the
spec.
  
  
  Robert - do you remember why we didn’t add this to
the specification? (6404550)
  

  
On Feb 21, 2018, at 4:44 PM, serguei.spit...@oracle.com
  wrote:


  
Hi Karen,
  
  Thank you for sorting this out!
  
  
  On 2/21/18 09:55, Karen Kinnear wrote:

 Dan,
  
  
  Thank you for all the background
digging. This is really helpful.
  
  
  Serguei - do you know what tests exist
for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the
open/test/jdk/java/lang/instrument:
  RedefineMethodAddInvoke*
  RedefineMethodDelInvoke*
  
  


  The way I read the source code - we
currently allow ADD and DELETE for
  PRIVATE OR STATIC OR FINAL methods. Did
I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
  if ((old_flags & JVM_ACC_PRIVATE) == 0
   // hack: private should be treated as final,
but alas
  || (old_flags &
(JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
 ) {
    // deleted methods must be private
    return
JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
  }

I read it as we allow ADD and DELETE for PRIVATE
&& (STATIC || FINAL) methods.
(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC
methods.)
As private should always be treated final then we can
simplify the above to:
  We allow and and delete PRIVATE INSTANCE or PRIVATE
STATIC methods
  which is equal to just "PRIVATE methods”.
  

  
  
  
  Sergeui - you are right - I misread this today.
And I played with the tests a bit - thanks Dan - and it
  matches the way you read this.


So today, private methods that are not marked as final in
  the source code can not be added -
I tried that variation by modifying the
  RedefineMethodAddInvokeTarget_1.java and changing
private final void myMethod1() to private void myMethod1()
  and got an UnsupportedOperationException.


So - private methods are not marked as ACC_FINAL today so I
  think the simplification doesn’t
apply, so we are left with the ability to ADD or DELETE
  PRIVATE && (STATIC || FINAL) methods  - at least
  that is what we support today.
  


Thank you for confirmation!
I suspected this because of the comment:
   // hack: private should be treated as final, but alas


  

  

  

  With the current implementation, I am
not sure if deletion works for private methods - do
we
  have a test for that? Or could you add
one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


  
  I tried the test. And it works because of the requirement that
  FINAL or STATIC are set,
which therefore means no vtable entry.

  


Good to know.


  

  

  

  Today we create a vtable entry for
private methods (my misunderstanding ~ 2006ish).
After discussions
  with David I no longer believe we need
those.
  Today,

Re: JVMTI retransformation and addition of private methods

[David Holmes]

On 22/02/2018 7:44 AM, serguei.spit...@oracle.com wrote:

Hi Karen,

Thank you for sorting this out!


On 2/21/18 09:55, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
RedefineMethodAddInvoke*
RedefineMethodDelInvoke*



The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
   if ((old_flags & JVM_ACC_PRIVATE) == 0
    // hack: private should be treated as final, but alas
   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
  ) {
     // deleted methods must be private
     return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
   }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) 
methods.

(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the above to:
   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
   which is equal to just "PRIVATE methods".


I read it as "PRIVATE" or "FINAL STATIC".

David
-

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.


Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit the 
assert above.

If we no longer need this then we have no problem.

Thanks,
Serguei


thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an 
alternate to the "native rebinding" technique that had been used 
previously. See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need 

Re: JVMTI retransformation and addition of private methods

[Karen Kinnear]

Sergeui,

You were right - I read the sources incorrectly. Would still help to understand 
both
the motivation and the reason to not add to the spec.

Robert - do you remember why we didn’t add this to the specification? (6404550)

> On Feb 21, 2018, at 4:44 PM, serguei.spit...@oracle.com wrote:
> 
> Hi Karen,
> 
> Thank you for sorting this out!
> 
> 
> On 2/21/18 09:55, Karen Kinnear wrote:
>> Dan,
>> 
>> Thank you for all the background digging. This is really helpful.
>> 
>> Serguei - do you know what tests exist for this behavior?
> 
> Dan already replied (thanks, Dan!)
> There are two tests in the open/test/jdk/java/lang/instrument:
>   RedefineMethodAddInvoke*
>   RedefineMethodDelInvoke*
> 
> 
>> The way I read the source code - we currently allow ADD and DELETE for
>> PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?
> The above does not look correct to me.
> We have the same check for both ADD and DELETE method:
>   if ((old_flags & JVM_ACC_PRIVATE) == 0
>// hack: private should be treated as final, but alas
>   || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
>  ) {
> // deleted methods must be private
> return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
>   }
> 
> I read it as we allow ADD and DELETE for PRIVATE && (STATIC || FINAL) methods.
> (Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
> As private should always be treated final then we can simplify the above to:
>   We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
>   which is equal to just "PRIVATE methods”.

Sergeui - you are right - I misread this today.
And I played with the tests a bit - thanks Dan - and it matches the way you 
read this.

So today, private methods that are not marked as final in the source code can 
not be added -
I tried that variation by modifying the RedefineMethodAddInvokeTarget_1.java 
and changing
private final void myMethod1() to private void myMethod1() and got an 
UnsupportedOperationException.

So - private methods are not marked as ACC_FINAL today so I think the 
simplification doesn’t
apply, so we are left with the ability to ADD or DELETE
  PRIVATE && (STATIC || FINAL) methods  - at least that is what we support 
today.
> 
>> With the current implementation, I am not sure if deletion works for private 
>> methods - do we
>> have a test for that? Or could you add one as part of this exercise?
> 
> Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.
I tried the test. And it works because of the requirement that FINAL or STATIC 
are set,
which therefore means no vtable entry.
> 
> 
>> Today we create a vtable entry for private methods (my misunderstanding ~ 
>> 2006ish). After discussions
>> with David I no longer believe we need those.
>> Today, klassVtable::adjust_method_entries has an assertion
>>   assert(!old_method->is_deleted(), “vtable methods may not be deleted”)
>> 
>> I may have read the code incorrectly - but I would expect to hit this 
>> assertion if you had a private
>> method you were deleting that was not final and not static.
>> 
>> option 1) we could explicitly tighten the restrictions to match what we have 
>> implemented
>> option 2) we could make this work by changing 
>> klassVtable.cpp::update_inherited_vtable
>>   handling of private fields to be done the way it handles final fields.
>> option 3) I read the code incorrectly?
> 
> If we create a vtable entry for private methods then we should hit the assert 
> above.
> If we no longer need this then we have no problem.
We do create a vtable entry for private methods; however if FINAL or STATIC is 
set, then
we do not create a vtable entry.

That is why we don’t ever get here.

thanks,
Karen
> 
> Thanks,
> Serguei
> 
>> thanks,
>> Karen
>> 
>>> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
>>> > wrote:
>>> 
>>> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
>>>  wrote:
 On 2/20/18 23:01, David Holmes wrote: 
> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>  wrote: 
>> Hi Karen and David, 
>> 
>> 
>> On 2/20/18 19:52, David Holmes wrote: 
>>> Hi Karen, 
>>> 
>>> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
 Folks, 
 
 As part of the Valhalla EG discussions for JVMTI changes for nestmates 
 (thank you Serguei and David), 
 IBM brought up a request that we update the JVMTI documentation to 
 reflect that we allow addition 
 of private methods. 
 
 Is there a reason we do not document this? I’m inviting those who were 
 involved at the time - please include 
 others if needed. 
>> 
>> I support documenting this in the JVMTI spec and had a plan to fix it in 
>> 11. 
>> However, it is not clear to me yet if we have a consensus on 

Re: Thread Native ID Access

[Martin Buchholz]

On Wed, Feb 21, 2018 at 2:40 PM, Jeremy Manson 
wrote:

>
> 1) Should the ID be a long?  The Hotspot thread dump prints it out as
> 0x%x, which is an unsigned hexadecimal integer.  The type hiding behind it
> is platform-dependent, though: a pid_t on Linux, an unsigned long on
> Windows, a thread_t on Solaris.  I could make it a String instead...
>

This is very similar to the question of what to do for  e.g. Process#pid
https://docs.oracle.com/javase/9/docs/api/java/lang/Process.html#pid--
Note the Posixy naming of the method and its long return type.
When I was thinking about this 10 years ago, I was expecting us to
represent it as a String inside the JVM, which would be more flexible, but
it hasn't caused any trouble so far.  Since thread ids are even more system
dependent, the case for String is stronger.  It's at least plausible that
there are no native threads on the platform

Re: Thread Native ID Access

[mandy chung]

I'm not comfortable for ThreadInfo to expose the implementation 
details.  What should we specify w.r.t. Java Thread mapping to native 
thread which is platform dependent.  Also how does it relate to the 
future fibers [1]?


Another alternative is for JDK specific diagnostic tools to do that 
mapping for you for example jcmd, rather than exposing it in the API.  I 
assume is that this is more about troubleshooting than on-going VM 
monitoring.


Mandy
[1] http://openjdk.java.net/projects/loom/

On 2/21/18 2:40 PM, Jeremy Manson wrote:

Hey folks,

I mentioned earlier in the thread about the ThreadInfo.from() bug that 
I found this because I was looking at fixing JDK-8154176, which 
proposes introducing native thread ids to Thread and ThreadInfo.


https://bugs.openjdk.java.net/browse/JDK-8154176

I have a prototype for it.  I have a couple of questions, though:

0) Does anyone object to this being done or my doing it?  I see that 
it already has an owner.


1) Should the ID be a long?  The Hotspot thread dump prints it out as 
0x%x, which is an unsigned hexadecimal integer.  The type hiding 
behind it is platform-dependent, though: a pid_t on Linux, an unsigned 
long on Windows, a thread_t on Solaris.  I could make it a String 
instead...


Jeremy

Thread Native ID Access

[Jeremy Manson]

Hey folks,

I mentioned earlier in the thread about the ThreadInfo.from() bug that I
found this because I was looking at fixing JDK-8154176, which proposes
introducing native thread ids to Thread and ThreadInfo.

https://bugs.openjdk.java.net/browse/JDK-8154176

I have a prototype for it.  I have a couple of questions, though:

0) Does anyone object to this being done or my doing it?  I see that it
already has an owner.

1) Should the ID be a long?  The Hotspot thread dump prints it out as 0x%x,
which is an unsigned hexadecimal integer.  The type hiding behind it is
platform-dependent, though: a pid_t on Linux, an unsigned long on Windows,
a thread_t on Solaris.  I could make it a String instead...

Jeremy

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

  
  
Hi Karen,
  
  Thank you for sorting this out!
  
  
  On 2/21/18 09:55, Karen Kinnear wrote:


  
  Dan,
  
  
  Thank you for all the background digging. This is
really helpful.
  
  
  Serguei - do you know what tests exist for this
behavior?


Dan already replied (thanks, Dan!)
There are two tests in the open/test/jdk/java/lang/instrument:
  RedefineMethodAddInvoke*
  RedefineMethodDelInvoke*
  
  


  The way I read the source code - we currently allow
ADD and DELETE for
  PRIVATE OR STATIC OR FINAL methods. Did I read that
correctly?

The above does not look correct to me.
We have the same check for both ADD and DELETE method:
  if ((old_flags & JVM_ACC_PRIVATE) == 0
   // hack: private should be treated as final, but alas
  || (old_flags & (JVM_ACC_FINAL|JVM_ACC_STATIC)) == 0
 ) {
    // deleted methods must be private
    return JVMTI_ERROR_UNSUPPORTED_REDEFINITION_METHOD_DELETED;
  }

I read it as we allow ADD and DELETE for PRIVATE && (STATIC
|| FINAL) methods.
(Rephrase: We allow PRIVATE FINAL or PRIVATE STATIC methods.)
As private should always be treated final then we can simplify the
above to:
  We allow and and delete PRIVATE INSTANCE or PRIVATE STATIC methods
  which is equal to just "PRIVATE methods".


  With the current implementation, I am not sure if
deletion works for private methods - do we
  have a test for that? Or could you add one as part
of this exercise?


Yes, we have one j.l.instrument test: RedefineMethodDelInvoke.sh.



  Today we create a vtable entry for private methods
(my misunderstanding ~ 2006ish). After discussions
  with David I no longer believe we need those.
  Today, klassVtable::adjust_method_entries has an
assertion
    assert(!old_method->is_deleted(), “vtable
methods may not be deleted”)
  
  
  I may have read the code incorrectly - but I would
expect to hit this assertion if you had a private
  method you were deleting that was not final and not
static.
  
  
  option 1) we could explicitly tighten the
restrictions to match what we have implemented
  option 2) we could make this work by changing
klassVtable.cpp::update_inherited_vtable
    handling of private fields to be done the way it
handles final fields.
  option 3) I read the code incorrectly?


If we create a vtable entry for private methods then we should hit
the assert above.
If we no longer need this then we have no problem.

Thanks,
Serguei


  thanks,
  Karen
  

  
On Feb 21, 2018, at 10:40 AM, Daniel D.
  Daugherty 
  wrote:


   On 2/21/18 2:45 AM, serguei.spit...@oracle.com
wrote:
On 2/20/18 23:01, David Holmes wrote: 
  On 21/02/2018 4:50
PM, serguei.spit...@oracle.com
wrote: 
Hi Karen and David,
  
  
  
  On 2/20/18 19:52, David Holmes wrote: 
  Hi Karen, 

On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
Folks, 
  
  As part of the Valhalla EG discussions for
  JVMTI changes for nestmates (thank you Serguei
  and David), 
  IBM brought up a request that we update the
  JVMTI documentation to reflect that we allow
  addition 
  of private methods. 
  
  Is there a reason we do not document this? I’m
  inviting those who were involved at the time -
  please include 
  others if needed. 

  
  
  I support documenting this in the JVMTI spec and
  had a plan to fix it in 11. 
  However, it is not clear to me yet if we have a
  consensus on it. 


I would like to see a detailed analysis of the
implications of allowing this. I _think_ it is safe
but ... 
  
  
  Valid concern. 
  Also, I'd love to collect more details 

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

  
  
Hi Dan,
  
  On 2/21/18 10:02, Daniel D. Daugherty wrote:


  
  On 2/21/18 12:55 PM, Karen Kinnear wrote:
   Dan,


Thank you for all the background digging. This is
  really helpful.


Serguei - do you know what tests exist for this
  behavior?
  
   
I mentioned a test that I found in the email that I attached to
my
previous reply:


  Subject: 
  Re: adding/deleting methods in JVM/TI
  RedefineClasses()/RetransformClasses()
  

  


  
  

  From: 
  "Daniel D. Daugherty" 
  
  

  Date: 
  12/17/12, 3:03 PM
  

  
  

  

  To: 
  Karen Kinnear 
  
  

  CC: 
  Mikael Vidstedt ,
  Serguei Spitsyn 
  

  
  
  Looks like I added a JLI/JPLIS test that exercises this
  "feature" 
  by calling the newly added method via reflection: 
  
      $ ls
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*
      RedefineMethodAddInvoke.sh 
      RedefineMethodAddInvokeAgent.java 
      RedefineMethodAddInvokeApp.java 
      RedefineMethodAddInvokeTarget.java 
      RedefineMethodAddInvokeTarget_1.java 
      RedefineMethodAddInvokeTarget_2.java 
  
  This test was added for the following bug: 
  
      6667089 3/3 multiple redefinitions of a class break
  reflection 
  
  All these little pieces of information keep coming back at
  me... 
  

I've added a comment to the JDK-8192936 with the most important
fragments
from this email exchange in a hope, it will stop this bad loop. :)


 Dan

I haven't checked to see if that test is still there or if any
additional tests have been added.
  

Thank you for pointing to this!

Also, there is a test for DELETE method in the
open/test/jdk/java/lang/instrument:
  RedefineMethodDelInvokeAgent.java
  RedefineMethodDelInvokeApp.java
  RedefineMethodDelInvoke.sh
  RedefineMethodDelInvokeTarget_1.java
  RedefineMethodDelInvokeTarget_2.java
  RedefineMethodDelInvokeTarget.java

Thanks,
Serguei

 
Dan


  
  


The way I read the source code - we currently
  allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read
  that correctly?


With the current implementation, I am not sure if
  deletion works for private methods - do we
have a test for that? Or could you add one as part
  of this exercise?


Today we create a vtable entry for private methods
  (my misunderstanding ~ 2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an
  assertion
  assert(!old_method->is_deleted(), “vtable
  methods may not be deleted”)


I may have read the code incorrectly - but I would
  expect to hit this assertion if you had a private
method you were deleting that was not final and
  not static.


option 1) we could explicitly tighten the
  restrictions to match what we have implemented
option 2) we could make this work by changing
  klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it
  handles final fields.
option 3) I read the code incorrectly?


thanks,
Karen

  

  On Feb 21, 2018, at 10:40 AM, Daniel D.
Daugherty 
wrote:
  
  
 On 2/21/18 2:45 AM, serguei.spit...@oracle.com
  wrote:
  On 2/20/18 23:01, David Holmes wrote: 
On 21/02/2018 4:50
  PM, serguei.spit...@oracle.com
  wrote: 
  Hi Karen and
David, 


On 2/20/18 19:52, David Holmes wrote: 
   

Re: JVMTI retransformation and addition of private methods

[Daniel D. Daugherty]

On 2/21/18 12:55 PM, Karen Kinnear wrote:

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?


I mentioned a test that I found in the email that I attached to my
previous reply:


Subject:
Re: adding/deleting methods in JVM/TI 
RedefineClasses()/RetransformClasses()


From:
"Daniel D. Daugherty" 
Date:
12/17/12, 3:03 PM

To:
Karen Kinnear 
CC:
Mikael Vidstedt , Serguei Spitsyn 




Looks like I added a JLI/JPLIS test that exercises this "feature"
by calling the newly added method via reflection:

    $ ls 
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*

    RedefineMethodAddInvoke.sh
    RedefineMethodAddInvokeAgent.java
    RedefineMethodAddInvokeApp.java
    RedefineMethodAddInvokeTarget.java
    RedefineMethodAddInvokeTarget_1.java
    RedefineMethodAddInvokeTarget_2.java

This test was added for the following bug:

    6667089 3/3 multiple redefinitions of a class break reflection

All these little pieces of information keep coming back at me...

Dan


I haven't checked to see if that test is still there or if any
additional tests have been added.

Dan




The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

With the current implementation, I am not sure if deletion works for 
private methods - do we

have a test for that? Or could you add one as part of this exercise?

Today we create a vtable entry for private methods (my 
misunderstanding ~ 2006ish). After discussions

with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this 
assertion if you had a private

method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what 
we have implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable

  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?

thanks,
Karen

On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
> wrote:


On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation 
to reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those 
who were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 
morphed into "Permit the adding or deleting of private 
final/static methods with redefine" - nor why those changes 
failed to make any change to the spec itself. It is also unclear 
whether the add/delete is restricted to final/static methods or 
any private method? I can see that the intent was to only allow 
something that would not perturb the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than 
the one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an 
alternate to the "native rebinding" technique that had been used 
previously. See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It 

Re: JVMTI retransformation and addition of private methods

[Karen Kinnear]

Dan,

Thank you for all the background digging. This is really helpful.

Serguei - do you know what tests exist for this behavior?

The way I read the source code - we currently allow ADD and DELETE for
PRIVATE OR STATIC OR FINAL methods. Did I read that correctly?

With the current implementation, I am not sure if deletion works for private 
methods - do we
have a test for that? Or could you add one as part of this exercise?

Today we create a vtable entry for private methods (my misunderstanding ~ 
2006ish). After discussions
with David I no longer believe we need those.
Today, klassVtable::adjust_method_entries has an assertion
  assert(!old_method->is_deleted(), “vtable methods may not be deleted”)

I may have read the code incorrectly - but I would expect to hit this assertion 
if you had a private
method you were deleting that was not final and not static.

option 1) we could explicitly tighten the restrictions to match what we have 
implemented
option 2) we could make this work by changing 
klassVtable.cpp::update_inherited_vtable
  handling of private fields to be done the way it handles final fields.
option 3) I read the code incorrectly?

thanks,
Karen

> On Feb 21, 2018, at 10:40 AM, Daniel D. Daugherty 
>  wrote:
> 
> On 2/21/18 2:45 AM, serguei.spit...@oracle.com 
>  wrote:
>> On 2/20/18 23:01, David Holmes wrote: 
>>> On 21/02/2018 4:50 PM, serguei.spit...@oracle.com 
>>>  wrote: 
 Hi Karen and David, 
 
 
 On 2/20/18 19:52, David Holmes wrote: 
> Hi Karen, 
> 
> On 21/02/2018 1:54 AM, Karen Kinnear wrote: 
>> Folks, 
>> 
>> As part of the Valhalla EG discussions for JVMTI changes for nestmates 
>> (thank you Serguei and David), 
>> IBM brought up a request that we update the JVMTI documentation to 
>> reflect that we allow addition 
>> of private methods. 
>> 
>> Is there a reason we do not document this? I’m inviting those who were 
>> involved at the time - please include 
>> others if needed. 
 
 I support documenting this in the JVMTI spec and had a plan to fix it in 
 11. 
 However, it is not clear to me yet if we have a consensus on it. 
>>> 
>>> I would like to see a detailed analysis of the implications of allowing 
>>> this. I _think_ it is safe but ... 
>> 
>> Valid concern. 
>> Also, I'd love to collect more details on the initial motivation to relax 
>> the JVMTI spec. 
>> Most likely we had no CCC/CSR filed on this change. 
>> 
>> 
> This issue is tracked by: 
> 
> https://bugs.openjdk.java.net/browse/JDK-8192936 
>  
> 
> "RI does not follow the JVMTI RedefineClasses spec that is too strict in 
> the definition" 
 
 Yes, this is the one. 
 Thank you, David, for posting the link. 
 
 
> As I wrote there ... It is not at all clear how JDK-6404550 morphed into 
> "Permit the adding or deleting of private final/static methods with 
> redefine" - nor why those changes failed to make any change to the spec 
> itself. It is also unclear whether the add/delete is restricted to 
> final/static methods or any private method? I can see that the intent was 
> to only allow something that would not perturb the vtable for existing 
> instances. 
 
 I agree, there is a confusion somewhere. 
 Is it possible, the JDK-6404550 in JIRA is a different bug than the one in 
 the Bugtraq system? 
 
 The JDK-6404550 in JIRA has a different synopsis: 
 https://bugs.openjdk.java.net/browse/JDK-6404550 
  
   Cannot implement late attach in NetBeans Profiler due to missing 
 functionality in JVMTI 
>>> 
>>> Digging deeper ... to fix the problem described in that bug they augmented 
>>> JVM TI to allow private method redefinition as an alternate to the "native 
>>> rebinding" technique that had been used previously. See the final comment 
>>> in: 
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-6341303 
>>>  
>>> 
>>> "JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep with 
>>> late attach" 
>>> 
>>> which was closed as a duplicate. 
>> 
>> Thank you for the point. 
>> This explains it. 
>> It seems, the bug synopsis was changed at some moment. 
> 
> The synopsis for 6404550 has never changed. Here's the subject line when
> it was created on 2006.03.27:
> 
> > CR 6404550 *HOT* Created P1 hotspot/jvmti Cannot implement late attach in 
> > NetBeans Profiler due to missing functionality in JVMTI
> 
> I think the confusion arises over comments like this in 6341303:
> 
>>  Robert Field 
>>  added a 
>> comment - 2006-05-04 11:54
>> BT2:EVALUATION
>> 
>> This can now be 

Re: Remove unused import proposal: in JdpController.java

[Martin Buchholz]

I've done my fair share of import cleanup, but friction is high enough that
it should be done in bulk.

If you can succeed in loading jdk sources into intellij, then you can get
intellij to find all the unused imports for you, e.g. in the jdk core
libraries.

On Wed, Feb 21, 2018 at 7:29 AM, Andrew Leonard  wrote:

> Hi,
> I would like to find a sponsor please for this simple tidy up of
> JdpController.java ?
> It contains an unused "import sun.management.VMManagement;" which is for a
> sun specific java.management class which this class used to use prior to
> jdk9.
> Thanks
> Andrew
>
> diff --git 
> a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
> b/src/jdk.management.agent/share/classes/sun/management/
> jdp/JdpController.java
> --- a/src/jdk.management.agent/share/classes/sun/management/
> jdp/JdpController.java
> +++ b/src/jdk.management.agent/share/classes/sun/management/
> jdp/JdpController.java
> @@ -34,7 +34,6 @@
>  import java.lang.reflect.Field;
>  import java.lang.reflect.Method;
>  import java.lang.UnsupportedOperationException;
> -import sun.management.VMManagement;
>
>  /**
>   * JdpController is responsible to create and manage a broadcast loop.
>
>
>
>
>
> 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
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>

Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

[mandy chung]

On 2/20/18 9:48 PM, Jeremy Manson wrote:
I think that's a much better approach (I didn't notice the validate 
method :) )


I think you may want to grab my test changes, or make some similar 
ones.  Presumably, the tests do not pass with your change.


I ran your test and it passed before posting it.


I think the API wording change is very slightly confusing.

    *       A {@code CompositeData} does not contain this attribute
      *       when representing a {@code ThreadInfo} of JDK 6 or
older version.
      *       This attribute will be set to {@link
Thread#NORM_PRIORITY}.


I'd probably say "In such cases" at the beginning of the second 
sentence.  Also, is there a rule about when you say JDK 6 and when you 
say Java 6?


Java SE 6 should be the proper terminology be used in this case.  I 
actually try to see if we can use @since instead.  I will take any pass 
on the spec and how it can tie with @since in the getter methods.


I will look at it later today.

Mandy


Jeremy

On Tue, Feb 20, 2018 at 6:41 PM, mandy chung > wrote:


Hi Jeremy,

Another approach is to change
ThreadInfoCompositeData::validateCompositeData
to validate the given CompositeData.   I also revised
ThreadInfoCompositeData to
return the default value of the attributes, if not present.

This is the patch:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.00/


What do you think?
Mandy


On 2/20/18 4:46 PM, Jeremy Manson wrote:

(I dropped serviceability-dev from this thread by mistake!  Oops!)

Okay. Here's the revised patch.  LMK if that's what you had in mind.

http://cr.openjdk.java.net/~jmanson/8198253/webrev.01/


On Fri, Feb 16, 2018 at 6:20 PM, mandy chung
> wrote:

Hi Jeremy,

lockedMonitors and lockedSynchronizers attribute are not optional if 
that's
the issue you try to resolve. I think the specification should be 
clarified.

ThreadInfo::from supports the three different versions for 
interoperability:
1. CompositeData for JDK 1.5 ThreadInfo with no lockedMonitors and
lockedSynchronizers attribute
2. CompositeData for JDK 6 ThreadInfo with lockedMonitors and 
lockedSynchronizers
attributes
3. CompositeData for JDK 9 ThreadInfo with lockedMonitors and 
lockedSynchronizers
    attributes and with daemon and priority attribute.

JMX client can connect to a running VM in any version and get back a
proper ThreadInfo.

If ThreadInfo::from is called with a CompositeData containing daemon and
priority attribute but lockedMonitors and lockedSynchronizers 
attributes are
absent then the given CompositeData is invalid and 
IllegalArgumentException
should be thrown.

Does this help?

Mandy


On 2/15/18 4:46 PM, Jeremy Manson wrote:

Hi folks,

Been a long time!  I'd like someone to do a code review. 
This is in code I wrote a few years ago, and got wrong.  At
the time, David Holmes, Staffan Larsen, and Mandy Chung
reviewed it.  It does mean that people using
ThreadInfo.from(CompositeData) may be getting the wrong
values out for ThreadInfo, so it is definitely worth fixing.

The patch below fixes the bug, but is a pretty questionable
approach.  Better approaches happily considered.

Patch:
http://cr.openjdk.java.net/~jmanson/8198253/webrev.00/


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


Thanks!

Jeremy

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

[mandy chung]

On 2/20/18 9:55 PM, Harsha Wardhana B wrote:


We cannot get rid of specifying options via -D. We have plenty of -D 
flags but very few have short-hand alternative via 
--start-management-agent. If management properties are specified by 
--start-management-agent, the options specified by -D are anyway 
overwritten if specified.


I have to see the specification of this feature and give further 
feedback.   If the new option does not accept all management properties, 
what is the benefit of this new option?


Mandy

Re: JVMTI retransformation and addition of private methods

[Daniel D. Daugherty]

On 2/21/18 2:45 AM, serguei.spit...@oracle.com wrote:

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix 
it in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb 
the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the 
one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate 
to the "native rebinding" technique that had been used previously. 
See the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It seems, the bug synopsis was changed at some moment.


The synopsis for 6404550 has never changed. Here's the subject line when
it was created on 2006.03.27:

> CR 6404550 *HOT* Created P1 hotspot/jvmti Cannot implement late 
attach in NetBeans Profiler due to missing functionality in JVMTI


I think the confusion arises over comments like this in 6341303:

rfield Robert Field 
 
added a comment - 2006-05-04 11:54

BT2:EVALUATION

This can now be accomplished with Java programming language 
instrumentation, via:


 6404550: missing late attach (JVM TI redefine) functionality
   Permit the adding or deleting of private final/static 
methods with redefine


Closing this bug as a duplicate.


That's just Robert's style for an sccs delta comment:

D 1.65.2.3 06/04/25 23:36:35 rfield 140 139 00023/00013/03263
MRs:
COMMENTS:
6404550: missing late attach (JVM TI redefine) functionality
    Add/delete private methods, continued: changes per review

Back in the ancient past we tried to include some brief
info about the change in the delta comment. This was one of many
deltas associated with 6404550.

Please see the attached email that I sent on 2012.12.17 about the
history behind this issue... (sent to Karen, Mikael V, and Serguei)

It seems I forwarded that same email to Coleen, Markus G and Serguei
back on 2014.05.20. Since Markus is on that thread, it must have had
something to do with research about JFR...

I need to do a detailed read thru my e-mail archive for 6404550 to
see if I can spot some clues about why we didn't do a spec update.

Dan




Thanks,
Serguei



David
-



Thanks,
Serguei



--
David



thanks,
Karen








--- Begin Message ---

Looks like I added a JLI/JPLIS test that exercises this "feature"
by calling the newly added method via reflection:

$ ls 
/work/shared/bug_hunt/jdk8/exp/test/java/lang/instrument/RedefineMethodAdd*

RedefineMethodAddInvoke.sh
RedefineMethodAddInvokeAgent.java
RedefineMethodAddInvokeApp.java
RedefineMethodAddInvokeTarget.java
RedefineMethodAddInvokeTarget_1.java
RedefineMethodAddInvokeTarget_2.java

This test was added for the following bug:

6667089 3/3 multiple redefinitions of a class break reflection

All these little pieces of information keep coming back at me... :-)

Dan



On 12/11/12 8:10 PM, Karen Kinnear wrote:

But what a wonderful way to bound the add/delete 

Remove unused import proposal: in JdpController.java

[Andrew Leonard]

Hi,
I would like to find a sponsor please for this simple tidy up of 
JdpController.java ?
It contains an unused "import sun.management.VMManagement;" which is for a 
sun specific java.management class which this class used to use prior to 
jdk9.
Thanks
Andrew

diff --git 
a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java 
b/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
--- 
a/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
+++ 
b/src/jdk.management.agent/share/classes/sun/management/jdp/JdpController.java
@@ -34,7 +34,6 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.UnsupportedOperationException;
-import sun.management.VMManagement;
 
 /**
  * JdpController is responsible to create and manage a broadcast loop.





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 


Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

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

[Roger Riggs]

Hi,

I'm a bit leary of command line arguments being special cased and the 
corresponding custom
mappings to system properties.   The convenience is fine but we need to 
keep the handling
out of native code so it is easier to maintain.  We don't have a Java 
API for processing

(VM) command line arguments so they are being shoehorned into properties.

$.02, Roger

On 2/21/2018 12:55 AM, Harsha Wardhana B wrote:



On Wednesday 21 February 2018 01:51 AM, mandy chung wrote:

The code review and CSR review can be in parallel.  For this case,
I agree with Kumar to have CSR written that would help the code
review. Please specify the behavior and its relationship with
jcmd and other relevant diagnosability tools.

ok.


On 2/20/18 6:41 AM, Kumar Srinivasan wrote:


What is the behavior when 
-Dcom.sun.management.jmxremote.port=1234 --start-management-agent 
port=2345 -Dcom.sun.management.jmxremote.port=3456?


What is the value of the system property 
com.sun.management.jmxremote.port at runtime?  What port number 
does the management server start with?
As said earlier, values set via new flags override values set by 
-D flags. So 2345 will be the value of 
com.sun.management.jmxremote.port. Added a test case to validate 
that.


VM options are the last one wins if same option specified multiple 
times.  In this case, it could cause confusion (the last -D option 
sets the value to 3456 but it's set to a different value).


Why not taking the simplest approach - when --start-management-agent 
is set, it does not accept mixing the old way (i.e. does not accept 
the management properties to be set via -D)?   This RFE is to make 
the command-line simpler and ease-of-use.  I don't see any downside 
to migrate entirely to the new form.
We cannot get rid of specifying options via -D. We have plenty of -D 
flags but very few have short-hand alternative via 
--start-management-agent. If management properties are specified by 
--start-management-agent, the options specified by -D are anyway 
overwritten if specified.


Mandy

Harsha

Re: PING: RFR: JDK-8153333: [REDO] STW phases at Concurrent GC should count in PerfCounter

[Yasumasa Suenaga]

PING: Could you review it?


   http://cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.07/


JBS: https://bugs.openjdk.java.net/browse/JDK-815
CSR: https://bugs.openjdk.java.net/browse/JDK-8196862


Yasumasa


On 2018/02/15 10:23, Yasumasa Suenaga wrote:

Hi all,

CSR for this issue [1] has been approved.
This webrev has been reviewed by Stefan, but we need one more
reviewer. Could you review it?

   http://cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.07/


Thanks,

Yasumasa


[1] https://bugs.openjdk.java.net/browse/JDK-8196862



2018-02-06 22:33 GMT+09:00 Yasumasa Suenaga :

Hi Stefan,


This looks good to me, will do some more testing while waiting for a
second reviewer and I can sponsor the change once it's ready to go.



Thanks! I'm waiting for second reviewer.


What should I do to get CSR approve?


In the bug system under "More" you can choose "Create CSR" which is the
first step. More information can be found on the wiki:
https://wiki.openjdk.java.net/display/csr/CSR+FAQs



I filed new CSR:
   https://bugs.openjdk.java.net/browse/JDK-8196862


Yasumasa



On 2018/02/06 21:55, Stefan Johansson wrote:




On 2018-02-06 06:10, Yasumasa Suenaga wrote:


Hi Stefan,


I agree, for G1 this should not be controlled. Maybe I was a bit
unclear, I
was wondering why we want to control it for CMS.


I said to remove -XX:EnableConcGCPerfCounter in two years ago. I've
missed it :-)


http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/017125.html

So I uploaded new webrev. This change includes copyright year updates.

http://cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.06/


Thanks Yasumasa,

This looks good to me, will do some more testing while waiting for a
second reviewer and I can sponsor the change once it's ready to go.



This change passes all tests on submit repo, and
:hotspot_serviceability :jdk_tools tests on my laptop.


http://java.se.oracle.com:10065/mdash/jobs/mach5-one-ysuenaga-JDK-815-20180206-0222-10428



If we do the change for CMS, we should
probably also do a CSR, but that should be fairly straight forward.


What should I do to get CSR approve?


In the bug system under "More" you can choose "Create CSR" which is the
first step. More information can be found on the wiki:
https://wiki.openjdk.java.net/display/csr/CSR+FAQs

Cheers,
Stefan



Thanks,

Yasumasa


2018-02-06 0:33 GMT+09:00 Stefan Johansson :



On 2018-02-03 06:40, Yasumasa Suenaga wrote:


On 2018/02/02 23:38, Stefan Johansson wrote:


Hi Yasumasa,

The changes doesn't apply clean on the latest jdk/hs, can you provide
an
updated webrev?



I uploaded webrev for jdk-hs:
cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.05/


Thanks, I've kicked off a testing job now to verify nothing unexpected
fails.




The testing done by the submit repo doesn't cover the tests you have
update so I plan to take the change for a spin and make sure the
correct
tests are run and verified in Mach 5.



I've also tested hotspot/jtreg/:hotspot_serviceability and
jdk/:jdk_tools
on my laptop.
I did not see any errors / failures which are related to this change.


I also ran some local tests on this and it looks good.





Also a question about the change. Why do we need a special flag for
CMS?
I see that the original bug report refers to the flag as being a way
to turn
on and off the feature but the current implementation only consider
the flag
for CMS.





http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/016774.html

Originally, STW phases (Remark and Cleanup) at G1 are not counted in
jstat
FGC column.
So I think we need not to control the behavior of PerfCounter for G1.


I agree, for G1 this should not be controlled. Maybe I was a bit
unclear, I
was wondering why we want to control it for CMS. I think either we
should
change the behavior without guarding it by a flag or just skip updating
CMS
(and leave the pauses in FGC). If we do the change for CMS, we should
probably also do a CSR, but that should be fairly straight forward.

I also found the old review thread where Jon M had the same comment
(removing the flag) and it looks like all agreed on that:

http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2016-March/017118.html

Thanks,
Stefan



Thanks,

Yasumasa



Thanks,
Stefan

On 2018-02-01 14:58, Yasumasa Suenaga wrote:


PING: Could you review and sponsor it?


http://cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.04/



This change has been passed Mach 5 via submit repo:


http://java.se.oracle.com:10065/mdash/jobs/mach5-one-ysuenaga-JDK-815-20180201-0805-10101


Thanks,

Yasumasa


On 2017/11/01 22:02, Yasumasa Suenaga wrote:


PING: Could you review and sponsor it?


http://cr.openjdk.java.net/~ysuenaga/JDK-815/webrev.04/



Also I need JPRT results of this change.
Could you cooperate?


Thanks,

Yasumasa


On 2017/09/27 0:08, Yasumasa Suenaga wrote:


Hi all,

I uploaded new webrev to be adapted to jdk10/hs:

Re: JVMTI retransformation and addition of private methods

[serguei.spit...@oracle.com]

On 2/20/18 23:01, David Holmes wrote:

On 21/02/2018 4:50 PM, serguei.spit...@oracle.com wrote:

Hi Karen and David,


On 2/20/18 19:52, David Holmes wrote:

Hi Karen,

On 21/02/2018 1:54 AM, Karen Kinnear wrote:

Folks,

As part of the Valhalla EG discussions for JVMTI changes for 
nestmates (thank you Serguei and David),
IBM brought up a request that we update the JVMTI documentation to 
reflect that we allow addition

of private methods.

Is there a reason we do not document this? I’m inviting those who 
were involved at the time - please include

others if needed.


I support documenting this in the JVMTI spec and had a plan to fix it 
in 11.

However, it is not clear to me yet if we have a consensus on it.


I would like to see a detailed analysis of the implications of 
allowing this. I _think_ it is safe but ...


Valid concern.
Also, I'd love to collect more details on the initial motivation to 
relax the JVMTI spec.

Most likely we had no CCC/CSR filed on this change.



This issue is tracked by:

https://bugs.openjdk.java.net/browse/JDK-8192936

"RI does not follow the JVMTI RedefineClasses spec that is too 
strict in the definition"


Yes, this is the one.
Thank you, David, for posting the link.


As I wrote there ... It is not at all clear how JDK-6404550 morphed 
into "Permit the adding or deleting of private final/static methods 
with redefine" - nor why those changes failed to make any change to 
the spec itself. It is also unclear whether the add/delete is 
restricted to final/static methods or any private method? I can see 
that the intent was to only allow something that would not perturb 
the vtable for existing instances.


I agree, there is a confusion somewhere.
Is it possible, the JDK-6404550 in JIRA is a different bug than the 
one in the Bugtraq system?


The JDK-6404550 in JIRA has a different synopsis:
https://bugs.openjdk.java.net/browse/JDK-6404550
  Cannot implement late attach in NetBeans Profiler due to 
missing functionality in JVMTI


Digging deeper ... to fix the problem described in that bug they 
augmented JVM TI to allow private method redefinition as an alternate 
to the "native rebinding" technique that had been used previously. See 
the final comment in:


https://bugs.openjdk.java.net/browse/JDK-6341303

"JVMTI Spec: Need a way how to rebind Object.wait and Thread.sleep 
with late attach"


which was closed as a duplicate.


Thank you for the point.
This explains it.
It seems, the bug synopsis was changed at some moment.

Thanks,
Serguei



David
-



Thanks,
Serguei



--
David



thanks,
Karen