Discussion:
RFR: JDK-8206330: Revisit com/sun/jdi/RedefineCrossEvent.java
Gary Adams
2018-10-17 16:46:07 UTC
Permalink
The RedefineCrossEvent test has been on been on the ProblemList for a
very long time.
In the past this test had some dependency on the Java EE modules, but
they were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.

Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Chris Plummer
2018-10-17 18:04:06 UTC
Permalink
Hi Gary,

Can this be on one line now:

  30  * @modules
  31  *          jdk.jdi

What are you blocking all jdk.* classes from redef when you pointed out
that only jdk.internal.* is an issue?

Is this also addressing JDK-8180804? If not, should it stay on the
problem list until JDK-8180804 is fixed?

thanks,

Chris
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for a
very long time.
In the past this test had some dependency on the Java EE modules, but
they  were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
  Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
  Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
g***@oracle.com
2018-10-17 19:40:44 UTC
Permalink
Post by Chris Plummer
Hi Gary,
  31  *          jdk.jdi
OK
Post by Chris Plummer
What are you blocking all jdk.* classes from redef when you pointed
out that only jdk.internal.* is an issue?
The file I'm modifying already has some blocking checks that look
for java., com, and sun., so it seemed appropriate that the jdk. prefix
was appropriate for this check.
Post by Chris Plummer
Is this also addressing JDK-8180804? If not, should it stay on the
problem list until JDK-8180804 is fixed?
I'll run through the usual testing to see if I can force the timeout.
Since the test has been on the ProblemList for a year and half
it is hard to know if this is an intermittent failure or not.

If I can't reproduce the failure, then I'd like to go ahead with this
change and see if it shows up again.

The original reason the test was placed on the ProblemList
was because of the removal of java ee modules, not because of the
timeout that was observed.
Post by Chris Plummer
thanks,
Chris
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for a
very long time.
In the past this test had some dependency on the Java EE modules, but
they  were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
  Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
  Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Chris Plummer
2018-10-17 19:58:53 UTC
Permalink
Post by g***@oracle.com
Post by Chris Plummer
Hi Gary,
  31  *          jdk.jdi
OK
Post by Chris Plummer
What are you blocking all jdk.* classes from redef when you pointed
out that only jdk.internal.* is an issue?
The file I'm modifying already has some blocking checks that look
for java., com, and sun., so it seemed appropriate that the jdk. prefix
was appropriate for this check.
Ok.
Post by g***@oracle.com
Post by Chris Plummer
Is this also addressing JDK-8180804? If not, should it stay on the
problem list until JDK-8180804 is fixed?
I'll run through the usual testing to see if I can force the timeout.
Since the test has been on the ProblemList for a year and half
it is hard to know if this is an intermittent failure or not.
If I can't reproduce the failure, then I'd like to go ahead with this
change and see if it shows up again.
Ok.
Post by g***@oracle.com
The original reason the test was placed on the ProblemList
was because of the removal of java ee modules, not because of the
timeout that was observed.
Yes, I realize that. But if the timeout is also somewhat new, it could
be problematic enough to warrant keeping it on the problem list.

thanks,

Chris
Post by g***@oracle.com
Post by Chris Plummer
thanks,
Chris
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for
a very long time.
In the past this test had some dependency on the Java EE modules,
but they  were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
  Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
  Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
s***@oracle.com
2018-10-17 23:26:52 UTC
Permalink
Hi Gary,

With the suggest tweaks below the fix looks good to me (modulo
suggestions from Alex in the following message).
I'm Okay with any approach related to the problem list.
It looks wrong to keep the test problem listed with the same bug id.
If we want to keep it then the bug id needs to be replaced with 8180804.

Thanks,
Serguei
Post by Chris Plummer
Post by g***@oracle.com
Post by Chris Plummer
Hi Gary,
  31  *          jdk.jdi
OK
Post by Chris Plummer
What are you blocking all jdk.* classes from redef when you pointed
out that only jdk.internal.* is an issue?
The file I'm modifying already has some blocking checks that look
for java., com, and sun., so it seemed appropriate that the jdk. prefix
was appropriate for this check.
Ok.
Post by g***@oracle.com
Post by Chris Plummer
Is this also addressing JDK-8180804? If not, should it stay on the
problem list until JDK-8180804 is fixed?
I'll run through the usual testing to see if I can force the timeout.
Since the test has been on the ProblemList for a year and half
it is hard to know if this is an intermittent failure or not.
If I can't reproduce the failure, then I'd like to go ahead with this
change and see if it shows up again.
Ok.
Post by g***@oracle.com
The original reason the test was placed on the ProblemList
was because of the removal of java ee modules, not because of the
timeout that was observed.
Yes, I realize that. But if the timeout is also somewhat new, it could
be problematic enough to warrant keeping it on the problem list.
thanks,
Chris
Post by g***@oracle.com
Post by Chris Plummer
thanks,
Chris
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for
a very long time.
In the past this test had some dependency on the Java EE modules,
but they  were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
  Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
  Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Gary Adams
2018-10-18 11:18:27 UTC
Permalink
So far I have not seen the timeout in my current test runs.
Since the test has been on the ProblemList for 1.5 years
we would not have seen any timeouts recently.

The test does run through a large number of tests that formerly
relied on java ee modules, so may have been running longer
in the past.

My plan is to use this changeset as my first direct submission.
If it does start timing out we can added back to the ProblemList
and perform some more investigation.
Post by Chris Plummer
Hi Gary,
With the suggest tweaks below the fix looks good to me (modulo
suggestions from Alex in the following message).
I'm Okay with any approach related to the problem list.
It looks wrong to keep the test problem listed with the same bug id.
If we want to keep it then the bug id needs to be replaced with 8180804.
Thanks,
Serguei
Post by Chris Plummer
Post by g***@oracle.com
Post by Chris Plummer
Hi Gary,
31 * jdk.jdi
OK
Post by Chris Plummer
What are you blocking all jdk.* classes from redef when you pointed
out that only jdk.internal.* is an issue?
The file I'm modifying already has some blocking checks that look
for java., com, and sun., so it seemed appropriate that the jdk. prefix
was appropriate for this check.
Ok.
Post by g***@oracle.com
Post by Chris Plummer
Is this also addressing JDK-8180804? If not, should it stay on the
problem list until JDK-8180804 is fixed?
I'll run through the usual testing to see if I can force the timeout.
Since the test has been on the ProblemList for a year and half
it is hard to know if this is an intermittent failure or not.
If I can't reproduce the failure, then I'd like to go ahead with this
change and see if it shows up again.
Ok.
Post by g***@oracle.com
The original reason the test was placed on the ProblemList
was because of the removal of java ee modules, not because of the
timeout that was observed.
Yes, I realize that. But if the timeout is also somewhat new, it
could be problematic enough to warrant keeping it on the problem list.
thanks,
Chris
Post by g***@oracle.com
Post by Chris Plummer
thanks,
Chris
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList
for a very long time.
In the past this test had some dependency on the Java EE modules,
but they were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Alex Menkov
2018-10-17 22:46:40 UTC
Permalink
Hi Gary,

- RedefineCrossEvent.java:

you can drop "@modules jdk.jdi" as well.
jdk.jdi is added to all test in the dir (it's specified in
test/jdk/com/sun/jdi/TEST.properties)

about InstanceFilter test - it's strange that RedefineCrossEvent fails
while InstanceFilter does not.
Maybe it's caused by different compiler options?
InstanceFilter.java contains:
@run compile -g InstanceFilter.java

and RedefineCrossEvent.java has:
@run build InstanceFilter

Note that all other classes in RedefineCrossEvent are compiled with "-g"
option.

--alex
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for a
very long time.
In the past this test had some dependency on the Java EE modules, but
they  were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
  Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
  Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Gary Adams
2018-10-18 11:25:59 UTC
Permalink
Post by Chris Plummer
Hi Gary,
jdk.jdi is added to all test in the dir (it's specified in
test/jdk/com/sun/jdi/TEST.properties)
Done.
Post by Chris Plummer
about InstanceFilter test - it's strange that RedefineCrossEvent fails
while InstanceFilter does not.
Maybe it's caused by different compiler options?
@run compile -g InstanceFilter.java
@run build InstanceFilter
Note that all other classes in RedefineCrossEvent are compiled with
"-g" option.
The InstanceFilter test has no problem running stand alone.
The RedefineCrossEvent test runs all the other tests in com/sun/jdi dir.
The problem is the redefine() that is called for each of the classes
encountered.
See TestScaffold.java.

In jdk9 there was an introduction of jdk.internal classes. In this
particular
test the jdk.internal.TerminatingThreadLocals class is observed, but the
redefine() processing has no compiled bytes to provide if the class were
redefined.
The test does not expect to process system classes and already includes
filters to exclude processing of "java.", "com.", and "sun." prefixed
classes.
Post by Chris Plummer
--alex
Post by Gary Adams
The RedefineCrossEvent test has been on been on the ProblemList for a
very long time.
In the past this test had some dependency on the Java EE modules, but
they were
deprecated for jdk9 and later removed completely in jdk11. This
changeset to restore
it, removes the corba module reference and blocks the redefine operations
for jdk.internal classes that presented an issue when the
RedefineCrossEvent
test launches the InstanceFilter test.
Issue: https://bugs.openjdk.java.net/browse/JDK-8206330
Webrev: http://cr.openjdk.java.net/~gadams/8206330/webrev/index.html
Loading...