Re: RFR 8135188: RunFinalizationTest.java Exception java.lang.Error: Test failure: Object was not finalized
On 10/15/15, 12:12 AM, Jaroslav Bachorik wrote:
On 15.10.2015 00:24, Martin Buchholz wrote:
Looks good, but cant-stop-nitpicking Martin feels compelled to point out
the typo here:
71 // his instance will be used to perform the
GC.run_finalization test
Hopefully, it is not necessary to repost the updated webrev. Or is it?
Not necessary for me.
Dan
-JB-
On Tue, Oct 13, 2015 at 11:47 PM, Jaroslav Bachorik
mailto:jaroslav.bacho...@oracle.com>>
wrote:
On 13.10.2015 20:12, Martin Buchholz wrote:
blockFinalizerThread looks buggy to me.
103 private static void blockFinalizerThread() throws
InterruptedException {
104 System.out.println("trying to block the finalizer
thread");
105 o1 = new MyObject();
106 o1 = null;
107 System.gc();
108 System.runFinalization();
109 finRunLatch.await();
110 }
Why are you calling System.runFinalization() ? You are trying
to block
the primary finalizer thread; you definitely don't want the
object to be
handled by the secondary finalizer thread.
Indeed. Even though this seems unlikely (didn't hit the problem in
500 repetitions) it will be better not to call
System.runFinalization() and just let System.gc() do its job
finalizing the collected instance.
---
51 } else {
52 System.out.println("finalizing the test
instance");
I would check :
else if (Thread.currentThread().getName().equals("Secondary
finalizer")) {
else fail(unexpected finalizer thread name)
If this ever happens it would mean that the finalizer logic has been
changed. Adding this check will make the test fail in such case and
force re-examination of the test logic. Sounds fair.
http://cr.openjdk.java.net/~jbachorik/8135188/webrev.04
Thanks!
-JB-
Re: jmx-dev RFR 7199353: Allow ConstructorProperties annotation from any package
On 14.10.2015 17:11, Jaroslav Bachorik wrote: On 14.10.2015 16:52, Mandy Chung wrote: On Oct 14, 2015, at 7:25 AM, Alan Bateman wrote: Hm, shouldn't we name the new annotation differently then? @ConstructorMapping ? It is not mandatory that we keep the actual name - we are changing the package anyway ... This may have been discussed previously, Mandy might know. I think at one point that jmx-dev was thinking about matching on any @CP property and that might have influenced the naming. I don’t recall any discussion on the name. The initial suggestion was to match any @CP. One benefit of keeping it @ConstructorProperties is for easy migration from java.beans to javax.management. I don’t have strong opinion if it should be a different name. Using a different name could prevent any confusion about @j.b.ConstructorProperties IMO, migration should be pretty straight forward with global replace even if we change the annotation name. Any objections to changing the annotation name to @ConstructorMapping to make it better distinguishable from @java.beans.ConstructorProperties ? -JB- -JB- Mandy
Re: jmx-dev RFR 7199353: Allow ConstructorProperties annotation from any package
On 15/10/2015 16:55, Jaroslav Bachorik wrote: Any objections to changing the annotation name to @ConstructorMapping to make it better distinguishable from @java.beans.ConstructorProperties ? Not from me. Do you mind updating the webrev so that we can see the updated javadoc? -Alan.
RFR 8139725: Backout escaped partial fix for JDK-7199353
Please, review this critical backout request Issue : https://bugs.openjdk.java.net/browse/JDK-8139725 Webrev: http://cr.openjdk.java.net/~jbachorik/8139725/webrev.00 By mistake a partial fix for JDK-7199353 has been pushed recently. It needs to be backed out ASAP. Thanks, -JB-
Re: RFR 8139725: Backout escaped partial fix for JDK-7199353
On 16/10/2015 06:02, Jaroslav Bachorik wrote: Please, review this critical backout request Issue : https://bugs.openjdk.java.net/browse/JDK-8139725 Webrev: http://cr.openjdk.java.net/~jbachorik/8139725/webrev.00 By mistake a partial fix for JDK-7199353 has been pushed recently. It needs to be backed out ASAP. Looks okay. -Alan
Re: RFR 8139725: Backout escaped partial fix for JDK-7199353
On 16.10.2015 07:13, Alan Bateman wrote: On 16/10/2015 06:02, Jaroslav Bachorik wrote: Please, review this critical backout request Issue : https://bugs.openjdk.java.net/browse/JDK-8139725 Webrev: http://cr.openjdk.java.net/~jbachorik/8139725/webrev.00 By mistake a partial fix for JDK-7199353 has been pushed recently. It needs to be backed out ASAP. Looks okay. Thanks! Pushing ... -JB- -Alan
