Re: RFR: JDK-8047769 SecureRandom should be more frugal with file descriptors

[Peter Levart]

Hi,

I have pushed this change. Thanks Brad, Alan and Chris for reviews.

Regards, Peter

On 01/06/2015 08:52 PM, Bradford Wetmore wrote:

I only looked at test, looks good to me.

I'd rarely ask to remove extra prints in tests. It adds initial debugging data points in case something breaks down the road.

Brad


On 1/4/2015 8:25 AM, Peter Levart wrote:

Hi Brad,

So I created another webrev (just removed the unneeded import and
left-over System.out.println from test):

http://cr.openjdk.java.net/~plevart/jdk9-dev/FileInputStreamPool.8047769/webrev.06/

I'll leave it here to rest for a couple of days and if no one objects,
I'll push it to jdk9-dev.

Thanks everybody for reviews and happy new year!

Regards, Peter

On 01/02/2015 11:58 PM, Bradford Wetmore wrote:

On 1/1/2015 12:22 PM, Peter Levart wrote:

Hi Brad,

Here's next webrev which tries to cover all your comments:

http://cr.openjdk.java.net/~plevart/jdk9-dev/FileInputStreamPool.8047769/webrev.04/

I downloaded the webrev.05 (Chris' later followup email) and ran it
through JPRT.  The only error was the previously seen -Dseed which is
not your problem.

Again, I only ran:

    jdk_lang, jdk_math, jdk_util, jdk_io, jdk_net, jdk_nio,
    jdk_security*, jdk_rmi, jdk_text, jdk_time, jdk_other, core_tools.

If you need anything else run, let me know.

Looks like you have a committer status, will you be pushing this?

I can, yes. As soon as we clear-out the remaining questions, right?

Yes.  The comments below are minor and shouldn't need another review
cycle.

I'm worried about the failure of the test you observed while running
from NetBeans. Perhaps a 0.5s wait is sometimes not enough for
ReferenceHandler thread to process (enqueue) a WeakReference. Since
there is already a facility in place to help ReferenceHandler thread
instead of wait for it, I used it in new version of the test.

BTW, there's now an unnecessary import from java.lang.AssertionError
added in webrev.04.

TEST RESULT: Failed. Compilation failed: Compilation failed

I changed the test to be self-contained now so one can run it without
testlib in classpath.

Thanks.  It's compiling fine now.

Two minor nits? SeedGenerator.java:  Lines 507/518

Done that too.

Thanks.

Maybe issue multiple reads to exercise in1 and in2?  e.g. 2 bytes of
in1, 4 bytes of in2, then 2 bytes of in1?

The 1st assert makes sure in1 == in2, so there's no point in invoking
the same instance via two aliases.

True.

IIRC, when I ran this under NetBeans last week, the second testCaching

didn't clear the WeakReference.

This should not happen any more now that the test is helping to enqueue

the WeakReferences instead of waiting for ReferenceHandler to enqueue
them.

Yes, that hit the refQueue.poll().

It's always interesting to work with core-libs folks, learn something
new everyday.  Mixing Lambdas/try-with.

I need a time-machine for your CFV/jdk8 Committer:

http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-August/002896.html

I vote yes.

The test can now fail only if System.gc() does not trigger
WeakReference processing in the VM. Can you give it a try on your
NetBeans environment?

One last comment.  It's now 2015.  ;)

Brad