> http://cr.openjdk.java.net/~jbachorik/8135188/webrev.01
test/serviceability/dcmd/gc/RunFinalizationTest.java
L53: OutputAnalyzer out =
ProcessTools.executeProcess(testAppPb);
L54: out.stderrShouldNotMatch("^" +
FinalizationRunner.FAILED + ".*")
L55: .stdoutShouldMatch("^" + FinalizationRunner.PASSED
+ ".*");
What's the return value from out.stderrShouldNotMatch()?
Does it simply return 'out'? Sorry, it's been a while
since I read the test framework...
No other comments.
test/serviceability/dcmd/gc/FinalizationRunner.java
No comments.
Thumbs up.
Dan
On 10/9/15 1:36 AM, Jaroslav Bachorik wrote:
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.bacho...@oracle.com <mailto:jaroslav.bacho...@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-
