RFR 8135188: RunFinalizationTest.java Exception java.lang.Error: Test failure: Object was not finalized

[Jaroslav Bachorik]

On 8.10.2015 18:56, Martin Buchholz wrote:
> Hi Jaroslav,
>
> we all keep writing finalization code like this... welcome to the club!
>
>    I think it would be better :
> - never use currentTimeMillis to measure elapsed time; use nanoTime instead

Ok. I suppose this would be because currentTimeMillis() is dependent on 
the OS time, right?

> - why use complex Phaser when simple CountDownLatch will do?

The logic is more complex than just waiting for the finalization to 
happen. I need to make sure the finalization happened due to 
GC.run_finalization command and not because of an ordinary GC run or JVM 
shutdown. I will update the test comments to make this clear.

>
> I just concidentally wrote this code, which I hope will be a model of
> best practice:
>
>      /** No guarantees, but effective in practice. */
>      private static void forceFullGc() {
>          CountDownLatch finalizeDone = new CountDownLatch(1);
>          WeakReference<?> ref = new WeakReference<Object>(new Object() {
>              protected void finalize() { finalizeDone.countDown(); }});
>          try {
>              for (int i = 0; i < 10; i++) {
>                  System.gc();
>                  if (finalizeDone.await(1L, SECONDS) && ref.get() == null) {
>                      System.runFinalization(); // try to pick up stragglers
>                      return;
>                  }
>              }
>          } catch (InterruptedException unexpected) {
>              throw new AssertionError("unexpected InterruptedException");
>          }
>          throw new AssertionError("failed to do a \"full\" gc");
>      }

This seems to be testing only the fact that the finalization has 
eventually been run - not that the finalization was actually provoked by 
System.gc() or System.runFinalization(). Unfortunately, this wouldn't 
suffice for my needs here.

-JB-

>
>
> On Thu, Oct 8, 2015 at 12:37 AM, Jaroslav Bachorik
> <jaroslav.bachorik at oracle.com <mailto:jaroslav.bachorik at oracle.com>> wrote:
>
>     Please, review the following test change
>
>     Issue : https://bugs.openjdk.java.net/browse/JDK-8135188
>     Webrev: http://cr.openjdk.java.net/~jbachorik/8135188/webrev.00
>
>     The problem is described in detail in the issue. This patch follows
>     the recommendation by Dan Daugherty and runs the part where
>     finalization is being requested in a shutdown handler as a separate
>     process. The test then checks the result of this separate process to
>     see whether any errors were reported. In order to simplify the logic
>     of the shutdown handler the failures are reported by simply printing
>     messages into stderr as opposed to throwing 'new Error()'.
>
>     The modified test is still passing on all supported platforms.
>
>     Thanks,
>
>     -JB-
>
>