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