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

[Jaroslav Bachorik]

On 9.10.2015 08:51, Jaroslav Bachorik wrote:
> 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.

Updated webrev addressing Dan's and Martin's concerns - 
http://cr.openjdk.java.net/~jbachorik/8135188/webrev.01

-JB-

>
> -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-
>>
>>
>