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

[Peter Levart]

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
>