Status of JEP159?

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Status of JEP159?

Thomas Stüfe-2
Hi all,

just a small question. 

While examining a crash in jvmti_GetClassMethods (jdk9) I noticed that I am able to successfully add and remove methods in a redefined class. 

But JEP159 is still only in "submitted" stage. Was this feature added for another JEP?

Thank you!

Kind Regards, Thomas
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

David Holmes
Hi Thomas,

On 16/10/2017 8:40 PM, Thomas Stüfe wrote:
> Hi all,
>
> just a small question.
>
> While examining a crash in jvmti_GetClassMethods (jdk9) I noticed that I
> am able to successfully add and remove methods in a redefined class.
>
> But JEP159 is still only in "submitted" stage. Was this feature added
> for another JEP?

According to the spec, you are not allowed to add/remove methods. How
did you add/remove them?

https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses

David
-----

> Thank you!
>
> Kind Regards, Thomas
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Robbin Ehn
Hi, if you use class file load hook you can add/remove public methods.
Since this is before the class have been published we don't know how it should look.
Whether this is according to spec or not, I have no clue.

Is it on CFLH ?

/Robbin

On 10/16/2017 01:20 PM, David Holmes wrote:

> Hi Thomas,
>
> On 16/10/2017 8:40 PM, Thomas Stüfe wrote:
>> Hi all,
>>
>> just a small question.
>>
>> While examining a crash in jvmti_GetClassMethods (jdk9) I noticed that I am able to successfully add and remove methods in a redefined class.
>>
>> But JEP159 is still only in "submitted" stage. Was this feature added for another JEP?
>
> According to the spec, you are not allowed to add/remove methods. How did you add/remove them?
>
> https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses
>
> David
> -----
>
>> Thank you!
>>
>> Kind Regards, Thomas
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Alan Bateman


On 16/10/2017 14:21, Robbin Ehn wrote:
> Hi, if you use class file load hook you can add/remove public methods.
> Since this is before the class have been published we don't know how
> it should look.
> Whether this is according to spec or not, I have no clue.
>
> Is it on CFLH ?
>
No issue adding or removing methods or making any other changes to the
class file in the CFLH but only for the initial load. The CFLH will be
re-run when the class is transformed (RetransformClasses) but that
cannot add/remove methods or do other schema changes.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Robbin Ehn
On 10/16/2017 03:31 PM, Alan Bateman wrote:

>
>
> On 16/10/2017 14:21, Robbin Ehn wrote:
>> Hi, if you use class file load hook you can add/remove public methods.
>> Since this is before the class have been published we don't know how it should look.
>> Whether this is according to spec or not, I have no clue.
>>
>> Is it on CFLH ?
>>
> No issue adding or removing methods or making any other changes to the class file in the CFLH but only for the initial load. The CFLH will be re-run when the class is
> transformed (RetransformClasses) but that cannot add/remove methods or do other schema changes.

There is actually an issue, we start all transformation with 'on' disk version.
If the agent that did the addition of a public method e.g. exits(removeTransformer) we can never re-transform it, instead we get:
"error method delete"
It have been suggested that we should use 'first published' class version as a baseline (the version after CFLH), but would break current agents (I assume).

This is my old patch for it:
diff -r 46a21d1c5f1c src/share/vm/prims/jvmtiExport.cpp
--- a/src/share/vm/prims/jvmtiExport.cpp    Fri Aug 12 14:12:55 2016 -0700
+++ b/src/share/vm/prims/jvmtiExport.cpp    Tue Aug 16 16:22:29 2016 +0200
@@ -661,7 +661,8 @@
        if (env->is_retransformable() && env->is_enabled(JVMTI_EVENT_CLASS_FILE_LOAD_HOOK)) {
          // retransformable agents need to cache the original class file
          // bytes if changes are made via the ClassFileLoadHook
-        post_to_env(env, true);
+        // cache the last version after load is completed, hence the published version
+        post_to_env(env, _load_kind != jvmti_class_load_kind_load);
        }
      }
    }

Is it a bug or work as intended? (or a bug we can't fix)

/Robbin

>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Thomas Stüfe-2
In reply to this post by David Holmes
Hi David,



On Mon, Oct 16, 2017 at 1:20 PM, David Holmes <[hidden email]> wrote:
Hi Thomas,

On 16/10/2017 8:40 PM, Thomas Stüfe wrote:
Hi all,

just a small question.

While examining a crash in jvmti_GetClassMethods (jdk9) I noticed that I am able to successfully add and remove methods in a redefined class.

But JEP159 is still only in "submitted" stage. Was this feature added for another JEP?

According to the spec, you are not allowed to add/remove methods. How did you add/remove them?

https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses

David
-----

I used jdb (redefine). I found that add/remove method worked for private methods, but not for public ones, so that explains it. I was examining a bug which now turned out to be a regression of https://bugs.openjdk.java.net/browse/JDK-8149743 - only in my case it was not a lambda method but just an ordinary private method.

Sorry for the noise.

Thomas

 


Thank you!

Kind Regards, Thomas

Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Alan Bateman
In reply to this post by Robbin Ehn
On 16/10/2017 14:55, Robbin Ehn wrote:

>
> There is actually an issue, we start all transformation with 'on' disk
> version.
> If the agent that did the addition of a public method e.g.
> exits(removeTransformer) we can never re-transform it, instead we get:
> "error method delete"
> It have been suggested that we should use 'first published' class
> version as a baseline (the version after CFLH), but would break
> current agents (I assume).
>
> :
> Is it a bug or work as intended? (or a bug we can't fix)
If all agents (or JVM TI environments) are retransformation capable then
retransformClasses should send the initial class file bytes (or the "on
disk" version as you termed it) to the CFLH of the first agent. If a
retransformation capable agent adds a method in the initial load then it
should add it again when called to retransform the class.

On the other hand, if there are retransformation incapable agents in
picture then the class file bytes sent to the CFLH of the first
retransformation capable agent will be the class bytes from the output
from the retransformation incapable agents. So if retransformation
incapable agent adds a method in the initial load then that method will
exist in the class bytes that the retransformation capable agents see
when they retransform.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Robbin Ehn
On 10/16/2017 05:44 PM, Alan Bateman wrote:

> On 16/10/2017 14:55, Robbin Ehn wrote:
>>
>> There is actually an issue, we start all transformation with 'on' disk version.
>> If the agent that did the addition of a public method e.g. exits(removeTransformer) we can never re-transform it, instead we get:
>> "error method delete"
>> It have been suggested that we should use 'first published' class version as a baseline (the version after CFLH), but would break current agents (I assume).
>>
>> :
>> Is it a bug or work as intended? (or a bug we can't fix)
> If all agents (or JVM TI environments) are retransformation capable then retransformClasses should send the initial class file bytes (or the "on disk" version as you termed
> it) to the CFLH of the first agent. If a retransformation capable agent adds a method in the initial load then it should add it again when called to retransform the class.
>
> On the other hand, if there are retransformation incapable agents in picture then the class file bytes sent to the CFLH of the first retransformation capable agent will be
> the class bytes from the output from the retransformation incapable agents. So if retransformation incapable agent adds a method in the initial load then that method will
> exist in the class bytes that the retransformation capable agents see when they retransform.

I see, in my case it's several CFLH agents with retransformation capability. The one that added the public method is removed and no longer called.
Leaving the other agents without the ability to retransform anymore since they get the class file bytes without the public method.

/Robbin

>
> -Alan
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

Alan Bateman
On 16/10/2017 16:59, Robbin Ehn wrote:
>
> I see, in my case it's several CFLH agents with retransformation
> capability. The one that added the public method is removed and no
> longer called.
> Leaving the other agents without the ability to retransform anymore
> since they get the class file bytes without the public method.
That case is tricky. Minimally we should put something in the spec and
javadoc to discourage disabling the event or removing the transformer
when the agent is retransformation capable and it has performed schema
changes or added/removed methods at initial load. There are several
technical solutions possible of course but hard to know whether it's
worth introducing more complexity for such rare cases.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

David Holmes
In reply to this post by Thomas Stüfe-2
On 17/10/2017 1:03 AM, Thomas Stüfe wrote:

> Hi David,
>
> On Mon, Oct 16, 2017 at 1:20 PM, David Holmes <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Thomas,
>
>     On 16/10/2017 8:40 PM, Thomas Stüfe wrote:
>
>         Hi all,
>
>         just a small question.
>
>         While examining a crash in jvmti_GetClassMethods (jdk9) I
>         noticed that I am able to successfully add and remove methods in
>         a redefined class.
>
>         But JEP159 is still only in "submitted" stage. Was this feature
>         added for another JEP?
>
>
>     According to the spec, you are not allowed to add/remove methods.
>     How did you add/remove them?
>
>     https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses
>     <https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses>
>
>     David
>     -----
>
>
> I used jdb (redefine). I found that add/remove method worked for private
> methods, but not for public ones, so that explains it. I was examining a
> bug which now turned out to be a regression of
> https://bugs.openjdk.java.net/browse/JDK-8149743 - only in my case it
> was not a lambda method but just an ordinary private method.
>
> Sorry for the noise.

It isn't noise. The spec prohibits adding/removing methods - period! It
doesn't make an exception for private methods (even if it may seem
reasonable to do so).

David

> Thomas
>
>
>
>         Thank you!
>
>         Kind Regards, Thomas
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Status of JEP159?

David Holmes
In reply to this post by Robbin Ehn
On 16/10/2017 11:21 PM, Robbin Ehn wrote:
> Hi, if you use class file load hook you can add/remove public methods.
> Since this is before the class have been published we don't know how it
> should look.
> Whether this is according to spec or not, I have no clue.

There's no special dispensation in the spec for redefinition at CFLH
time AFAICS, so this seems like a bug to me!

David

> Is it on CFLH ?
>
> /Robbin
>
> On 10/16/2017 01:20 PM, David Holmes wrote:
>> Hi Thomas,
>>
>> On 16/10/2017 8:40 PM, Thomas Stüfe wrote:
>>> Hi all,
>>>
>>> just a small question.
>>>
>>> While examining a crash in jvmti_GetClassMethods (jdk9) I noticed
>>> that I am able to successfully add and remove methods in a redefined
>>> class.
>>>
>>> But JEP159 is still only in "submitted" stage. Was this feature added
>>> for another JEP?
>>
>> According to the spec, you are not allowed to add/remove methods. How
>> did you add/remove them?
>>
>> https://docs.oracle.com/javase/9/docs/specs/jvmti.html#RedefineClasses
>>
>> David
>> -----
>>
>>> Thank you!
>>>
>>> Kind Regards, Thomas