Incompatible change to java.lang.instrument.Instrumentation

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

Incompatible change to java.lang.instrument.Instrumentation

Sven Reimers
Hi all,

as part of the process of the code donation from Oracle to Apache for
NetBeans I tried a compilation of the source code using idk 9-ea+173. The
compilation fails due to an added method boolean isModifiableModule(Module)
in java.lang.instrument.Instrumentation.

Should the newly added method on the interface Instrumentation have a
default implementation to make this change source code compatible from 8 to
9, e.g.

default boolean isModifiableModule(Module module) {

    return false;

}


Is it already to late for changing this before JDK 9 release?

Thanks for considering

Sven

--
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Alan Bateman
On 16/06/2017 06:54, Sven Reimers wrote:

> Hi all,
>
> as part of the process of the code donation from Oracle to Apache for
> NetBeans I tried a compilation of the source code using idk 9-ea+173. The
> compilation fails due to an added method boolean isModifiableModule(Module)
> in java.lang.instrument.Instrumentation.
>
> Should the newly added method on the interface Instrumentation have a
> default implementation to make this change source code compatible from 8 to
> 9, e.g.
>
> default boolean isModifiableModule(Module module) {
>
>      return false;
>
> }
>
>
> Is it already to late for changing this before JDK 9 release?
>
You are correct that adding abstract methods to Instrumentation is an
incompatible change but the long term assumption in this area has always
been it wouldn't be implemented outside of the java.instrument module.
There were 4 abstract methods added to it in Java SE 6 and I don't
recall anyone screaming (pre-dates default methods of course).

So can you say what NetBeans is doing? Is this mocking, maybe
forwarding/intercepting? If there is a compelling reasons then we could
try to see about changing redefineModule and the other new methods to be
default methods but it is late and there is more to this that changing
them to be default methods.

-Alan

Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Sven Reimers
Hi Alan,

the code in question I am looking at is in

 o.n.bootstrap/src/org/netbeans/NbInstrumentation.java

So this looks more like some basic stuff baked deep into NetBeans core...

cc'Ing Jaroslav Tulach who wrote this - he should have some more details..
else I will try to figure this out.

Thanks for looking into this

Sven


On Fri, Jun 16, 2017 at 8:11 AM, Alan Bateman <[hidden email]>
wrote:

> On 16/06/2017 06:54, Sven Reimers wrote:
>
>> Hi all,
>>
>> as part of the process of the code donation from Oracle to Apache for
>> NetBeans I tried a compilation of the source code using idk 9-ea+173. The
>> compilation fails due to an added method boolean
>> isModifiableModule(Module)
>> in java.lang.instrument.Instrumentation.
>>
>> Should the newly added method on the interface Instrumentation have a
>> default implementation to make this change source code compatible from 8
>> to
>> 9, e.g.
>>
>> default boolean isModifiableModule(Module module) {
>>
>>      return false;
>>
>> }
>>
>>
>> Is it already to late for changing this before JDK 9 release?
>>
>> You are correct that adding abstract methods to Instrumentation is an
> incompatible change but the long term assumption in this area has always
> been it wouldn't be implemented outside of the java.instrument module.
> There were 4 abstract methods added to it in Java SE 6 and I don't recall
> anyone screaming (pre-dates default methods of course).
>
> So can you say what NetBeans is doing? Is this mocking, maybe
> forwarding/intercepting? If there is a compelling reasons then we could try
> to see about changing redefineModule and the other new methods to be
> default methods but it is late and there is more to this that changing them
> to be default methods.
>
> -Alan
>
>


--
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
                              Desktop Java:
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Jaroslav Tulach
On pátek 16. června 2017 8:17:51 CEST Sven Reimers wrote:

> Hi Alan,
>
> the code in question I am looking at is in
>
>  o.n.bootstrap/src/org/netbeans/NbInstrumentation.java
>
> So this looks more like some basic stuff baked deep into NetBeans core...
>
> cc'Ing Jaroslav Tulach who wrote this - he should have some more details..
> else I will try to figure this out.
>
> Thanks for looking into this
>
> Sven
>
>
> On Fri, Jun 16, 2017 at 8:11 AM, Alan Bateman <[hidden email]>
>
> wrote:
> > On 16/06/2017 06:54, Sven Reimers wrote:
> >> Hi all,
> >>
> >> as part of the process of the code donation from Oracle to Apache for
> >> NetBeans I tried a compilation of the source code using idk 9-ea+173. The
> >> compilation fails due to an added method boolean
> >> isModifiableModule(Module)
> >> in java.lang.instrument.Instrumentation.
> >>
> >> Should the newly added method on the interface Instrumentation have a
> >> default implementation to make this change source code compatible from 8
> >> to
> >> 9, e.g.
> >>
> >> default boolean isModifiableModule(Module module) {
> >>
> >>      return false;
> >>
> >> }
> >>
> >>
> >> Is it already to late for changing this before JDK 9 release?
> >>
> >> You are correct that adding abstract methods to Instrumentation is an
> >
> > incompatible change but the long term assumption in this area has always
> > been it wouldn't be implemented outside of the java.instrument module.
> > There were 4 abstract methods added to it in Java SE 6 and I don't recall
> > anyone screaming (pre-dates default methods of course).

Looks like I have implemented the code when compiling against JDK7.

> > So can you say what NetBeans is doing? Is this mocking, maybe
> > forwarding/intercepting?

See https://netbeans.org/bugzilla/show_bug.cgi?id=237919

> > If there is a compelling reasons then we could
> > try
> > to see about changing redefineModule and the other new methods to be
> > default methods but it is late and there is more to this that changing
> > them
> > to be default methods.

-jt

Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Alan Bateman
On 16/06/2017 08:40, Jaroslav Tulach wrote:
> :
>>> So can you say what NetBeans is doing? Is this mocking, maybe
>>> forwarding/intercepting?
> See https://netbeans.org/bugzilla/show_bug.cgi?id=237919
>
If I read this correctly then NBInstrumentation is only interested in
the addTransformer methods. If non-test code expecting an
Instrumentation object gets a reference to the NBInstrumentation
implementation then it will deeply disappointed when it finds that
methods such as getObjectSize, appendToBootstrapClassLoaderSearch, etc.
don't implement the spec. If it wrapped or forwarded to a "real"
Instrumentation object then it might be more compelling.

However point taken and we'll have to see whether it makes sense to make
these default implementations (which has spec implications so I can't
guarantee that this can be done at this late stage in JDK 9).

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

Re: Incompatible change to java.lang.instrument.Instrumentation

Peter Levart
In reply to this post by Jaroslav Tulach
Hi Sven,

Just an idea...

What NetBeans could probably do is to create a special sub-interface of
java.lang.instrument.Instrumentation in its bootstrap jar file (the jar
passed to class-path) and make that jar file MR jar with two versions of
say org.netbeans.core.Instrumentation interfaces extending
java.lang.instrument.Instrumentation. The one for pre-9 JDK would have
an empty body while the one for JDK 9 would override the newly
introduced abstract methods with default implementations. NetBeans
implementation classes would then implement
org.netbeans.core.Instrumentation instead of
java.lang.instrument.Instrumentation.

Regards, Peter

On 06/16/2017 09:40 AM, Jaroslav Tulach wrote:

> On pátek 16. června 2017 8:17:51 CEST Sven Reimers wrote:
>> Hi Alan,
>>
>> the code in question I am looking at is in
>>
>>   o.n.bootstrap/src/org/netbeans/NbInstrumentation.java
>>
>> So this looks more like some basic stuff baked deep into NetBeans core...
>>
>> cc'Ing Jaroslav Tulach who wrote this - he should have some more details..
>> else I will try to figure this out.
>>
>> Thanks for looking into this
>>
>> Sven
>>
>>
>> On Fri, Jun 16, 2017 at 8:11 AM, Alan Bateman <[hidden email]>
>>
>> wrote:
>>> On 16/06/2017 06:54, Sven Reimers wrote:
>>>> Hi all,
>>>>
>>>> as part of the process of the code donation from Oracle to Apache for
>>>> NetBeans I tried a compilation of the source code using idk 9-ea+173. The
>>>> compilation fails due to an added method boolean
>>>> isModifiableModule(Module)
>>>> in java.lang.instrument.Instrumentation.
>>>>
>>>> Should the newly added method on the interface Instrumentation have a
>>>> default implementation to make this change source code compatible from 8
>>>> to
>>>> 9, e.g.
>>>>
>>>> default boolean isModifiableModule(Module module) {
>>>>
>>>>       return false;
>>>>
>>>> }
>>>>
>>>>
>>>> Is it already to late for changing this before JDK 9 release?
>>>>
>>>> You are correct that adding abstract methods to Instrumentation is an
>>> incompatible change but the long term assumption in this area has always
>>> been it wouldn't be implemented outside of the java.instrument module.
>>> There were 4 abstract methods added to it in Java SE 6 and I don't recall
>>> anyone screaming (pre-dates default methods of course).
> Looks like I have implemented the code when compiling against JDK7.
>
>>> So can you say what NetBeans is doing? Is this mocking, maybe
>>> forwarding/intercepting?
> See https://netbeans.org/bugzilla/show_bug.cgi?id=237919
>
>>> If there is a compelling reasons then we could
>>> try
>>> to see about changing redefineModule and the other new methods to be
>>> default methods but it is late and there is more to this that changing
>>> them
>>> to be default methods.
> -jt
>

Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Sven Reimers
Hi Peter,

nice idea. The only drawback is this requires JDK 9 for a build. What I
wanted to check is if I can compile the same unchanged source tree with JDK
8 and 9...

Since there is not yet a hard requirement for JDK 9 compilation, i.e. this
is just an experiment to figure out things that may be broken and need
work, I would not see any actions on NetBeans side at the moment. I was
just astonished about the breaking change naively assuming that default
methods should be an easy fix.

Thanks for the hint, waiting what Alan comes up with in the meantime
(probably more projects could be hit by the problem)

@Alan: Shall I file a bug?

Thanks for the fast responses

Sven

On Fri, Jun 16, 2017 at 10:21 AM, Peter Levart <[hidden email]>
wrote:

> Hi Sven,
>
> Just an idea...
>
> What NetBeans could probably do is to create a special sub-interface of
> java.lang.instrument.Instrumentation in its bootstrap jar file (the jar
> passed to class-path) and make that jar file MR jar with two versions of
> say org.netbeans.core.Instrumentation interfaces extending
> java.lang.instrument.Instrumentation. The one for pre-9 JDK would have an
> empty body while the one for JDK 9 would override the newly introduced
> abstract methods with default implementations. NetBeans implementation
> classes would then implement org.netbeans.core.Instrumentation instead of
> java.lang.instrument.Instrumentation.
>
> Regards, Peter
>
>
> On 06/16/2017 09:40 AM, Jaroslav Tulach wrote:
>
>> On pátek 16. června 2017 8:17:51 CEST Sven Reimers wrote:
>>
>>> Hi Alan,
>>>
>>> the code in question I am looking at is in
>>>
>>>   o.n.bootstrap/src/org/netbeans/NbInstrumentation.java
>>>
>>> So this looks more like some basic stuff baked deep into NetBeans core...
>>>
>>> cc'Ing Jaroslav Tulach who wrote this - he should have some more
>>> details..
>>> else I will try to figure this out.
>>>
>>> Thanks for looking into this
>>>
>>> Sven
>>>
>>>
>>> On Fri, Jun 16, 2017 at 8:11 AM, Alan Bateman <[hidden email]>
>>>
>>> wrote:
>>>
>>>> On 16/06/2017 06:54, Sven Reimers wrote:
>>>>
>>>>> Hi all,
>>>>>
>>>>> as part of the process of the code donation from Oracle to Apache for
>>>>> NetBeans I tried a compilation of the source code using idk 9-ea+173.
>>>>> The
>>>>> compilation fails due to an added method boolean
>>>>> isModifiableModule(Module)
>>>>> in java.lang.instrument.Instrumentation.
>>>>>
>>>>> Should the newly added method on the interface Instrumentation have a
>>>>> default implementation to make this change source code compatible from
>>>>> 8
>>>>> to
>>>>> 9, e.g.
>>>>>
>>>>> default boolean isModifiableModule(Module module) {
>>>>>
>>>>>       return false;
>>>>>
>>>>> }
>>>>>
>>>>>
>>>>> Is it already to late for changing this before JDK 9 release?
>>>>>
>>>>> You are correct that adding abstract methods to Instrumentation is an
>>>>>
>>>> incompatible change but the long term assumption in this area has always
>>>> been it wouldn't be implemented outside of the java.instrument module.
>>>> There were 4 abstract methods added to it in Java SE 6 and I don't
>>>> recall
>>>> anyone screaming (pre-dates default methods of course).
>>>>
>>> Looks like I have implemented the code when compiling against JDK7.
>>
>> So can you say what NetBeans is doing? Is this mocking, maybe
>>>> forwarding/intercepting?
>>>>
>>> See https://netbeans.org/bugzilla/show_bug.cgi?id=237919
>>
>> If there is a compelling reasons then we could
>>>> try
>>>> to see about changing redefineModule and the other new methods to be
>>>> default methods but it is late and there is more to this that changing
>>>> them
>>>> to be default methods.
>>>>
>>> -jt
>>
>>
>


--
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
                              Desktop Java:
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers
Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Peter Levart
Hi Sven,

On 06/16/2017 11:08 AM, Sven Reimers wrote:
> Hi Peter,
>
> nice idea. The only drawback is this requires JDK 9 for a build. What
> I wanted to check is if I can compile the same unchanged source tree
> with JDK 8 and 9...

Another trick that comes to my mind is to dynamically generate a
java.lang.instrument.Instrumentation implementation using
java.lang.reflect.Proxy and create an InvocationHandler as a substitute
for NbInstrumentation rather than directly implementing the interface...

Regards, Peter

>
> Since there is not yet a hard requirement for JDK 9 compilation, i.e.
> this is just an experiment to figure out things that may be broken and
> need work, I would not see any actions on NetBeans side at the moment.
> I was just astonished about the breaking change naively assuming that
> default methods should be an easy fix.
>
> Thanks for the hint, waiting what Alan comes up with in the meantime
> (probably more projects could be hit by the problem)
>
> @Alan: Shall I file a bug?
>
> Thanks for the fast responses
>
> Sven
>
> On Fri, Jun 16, 2017 at 10:21 AM, Peter Levart <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Hi Sven,
>
>     Just an idea...
>
>     What NetBeans could probably do is to create a special
>     sub-interface of java.lang.instrument.Instrumentation in its
>     bootstrap jar file (the jar passed to class-path) and make that
>     jar file MR jar with two versions of say
>     org.netbeans.core.Instrumentation interfaces extending
>     java.lang.instrument.Instrumentation. The one for pre-9 JDK would
>     have an empty body while the one for JDK 9 would override the
>     newly introduced abstract methods with default implementations.
>     NetBeans implementation classes would then implement
>     org.netbeans.core.Instrumentation instead of
>     java.lang.instrument.Instrumentation.
>
>     Regards, Peter
>
>
>     On 06/16/2017 09:40 AM, Jaroslav Tulach wrote:
>
>         On pátek 16. června 2017 8:17:51 CEST Sven Reimers wrote:
>
>             Hi Alan,
>
>             the code in question I am looking at is in
>
>               o.n.bootstrap/src/org/netbeans/NbInstrumentation.java
>
>             So this looks more like some basic stuff baked deep into
>             NetBeans core...
>
>             cc'Ing Jaroslav Tulach who wrote this - he should have
>             some more details..
>             else I will try to figure this out.
>
>             Thanks for looking into this
>
>             Sven
>
>
>             On Fri, Jun 16, 2017 at 8:11 AM, Alan Bateman
>             <[hidden email] <mailto:[hidden email]>>
>
>             wrote:
>
>                 On 16/06/2017 06:54, Sven Reimers wrote:
>
>                     Hi all,
>
>                     as part of the process of the code donation from
>                     Oracle to Apache for
>                     NetBeans I tried a compilation of the source code
>                     using idk 9-ea+173. The
>                     compilation fails due to an added method boolean
>                     isModifiableModule(Module)
>                     in java.lang.instrument.Instrumentation.
>
>                     Should the newly added method on the interface
>                     Instrumentation have a
>                     default implementation to make this change source
>                     code compatible from 8
>                     to
>                     9, e.g.
>
>                     default boolean isModifiableModule(Module module) {
>
>                           return false;
>
>                     }
>
>
>                     Is it already to late for changing this before JDK
>                     9 release?
>
>                     You are correct that adding abstract methods to
>                     Instrumentation is an
>
>                 incompatible change but the long term assumption in
>                 this area has always
>                 been it wouldn't be implemented outside of the
>                 java.instrument module.
>                 There were 4 abstract methods added to it in Java SE 6
>                 and I don't recall
>                 anyone screaming (pre-dates default methods of course).
>
>         Looks like I have implemented the code when compiling against
>         JDK7.
>
>                 So can you say what NetBeans is doing? Is this
>                 mocking, maybe
>                 forwarding/intercepting?
>
>         See https://netbeans.org/bugzilla/show_bug.cgi?id=237919
>         <https://netbeans.org/bugzilla/show_bug.cgi?id=237919>
>
>                 If there is a compelling reasons then we could
>                 try
>                 to see about changing redefineModule and the other new
>                 methods to be
>                 default methods but it is late and there is more to
>                 this that changing
>                 them
>                 to be default methods.
>
>         -jt
>
>
>
>
>
> --
> Sven Reimers
>
> * Senior Expert Software Architect
> * Java Champion
> * NetBeans Dream Team Member: http://dreamteam.netbeans.org
> * Community Leader  NetBeans: http://community.java.net/netbeans
>                               Desktop Java:
> http://community.java.net/javadesktop
> * JUG Leader JUG Bodensee: http://www.jug-bodensee.de
> * Duke's Choice Award Winner 2009
>
> * XING: https://www.xing.com/profile/Sven_Reimers8
> * LinkedIn: http://www.linkedin.com/in/svenreimers

Reply | Threaded
Open this post in threaded view
|

Re: Incompatible change to java.lang.instrument.Instrumentation

Alan Bateman
In reply to this post by Sven Reimers
On 16/06/2017 10:08, Sven Reimers wrote:

> Hi Peter,
>
> nice idea. The only drawback is this requires JDK 9 for a build. What I
> wanted to check is if I can compile the same unchanged source tree with JDK
> 8 and 9...
>
> Since there is not yet a hard requirement for JDK 9 compilation, i.e. this
> is just an experiment to figure out things that may be broken and need
> work, I would not see any actions on NetBeans side at the moment. I was
> just astonished about the breaking change naively assuming that default
> methods should be an easy fix.
>
> Thanks for the hint, waiting what Alan comes up with in the meantime
> (probably more projects could be hit by the problem)
Hard to say. We've had new abstract methods on Instrumentation in the EA
builds for a  long time (and in main line JDK 9 EA builds since 3/2016)
and it hasn't come up before now (at least not to my knowledge).
>
> @Alan: Shall I file a bug?
>
Okay.