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

[Bradford Wetmore]

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