RFR [8038333] java/lang/ref/EarlyTimeout.java failed

[Peter Levart]

On 03/27/2014 02:36 PM, Ivan Gerasimov wrote:
> Thank you Peter!
>
>> There could be a little delay (say half the timeout: 500ms) specified 
>> after main thread returns from the startedSignal.await(); and before 
>> setting referent = null; and doing System.gc(). This would decrease 
>> the chance that the reference is enqueued before EarlyTimeout threads 
>> enter queue.remove(1000), thus making the test more reliable in 
>> failing with unpatched code.
>>
> Yes, you're right.
> I've checked the test against jdk9-b01 (no fix for 6853696 yet).
> It gives 2.5% of false negatives (i.e. in 5 out of 200 runs the 
> reference was enqueued before calling to remove()).
> With an additional delay of TIMEOUT / 2 this number dropped to 0%.
>
> With jdk9-b05 (with fix for 6853696) no false positives were shown 
> with or without this additional delay.
>
>> Now even if the referent is released and System.gc() is called, that 
>> does not guarantee that a WeakReference is going to be enqueued 
>> before the EarlyTimeout threads timeout and the result could as well 
>> be 0 collected references. To increase the chance that the reference 
>> is enqueued in a timely manner, main thread could, immediately after 
>> System.gc(), call:
>>
>> SharedSecrets.getJavaLangRefAccess().tryHandlePendingReference();
>>
>> (since SharedSecrets is in sun.misc protected package, 
>> JavaLangRefAccess instance would have to be obtained using reflection 
>> unfortunately).
>>
>
> I would prefer not to complicate the test too much, if you don't mind. 
> I think the test already shows reliable reproducibility.
>
>>> I suggest to return to the very first trivial fix:
>>> http://cr.openjdk.java.net/~igerasim/8038333/1/webrev/
>>
>> But this webrev *is moving startedSignal.await() after System.gc()* ...
>>
>
> Oops, sorry. It was meant to be /8038333/*0*/webrev/, of course!
>
> Now, I updated the webrev with the additional delay as you suggested:
> http://cr.openjdk.java.net/~igerasim/8038333/2/webrev/
>
> Would you please have a look?

That looks good. So no warning if nonNullRefCount == 0 ... It is 
probably not particularly useful if nobody is going to look at it, right?

Regards, Peter

>
> Sincerely yours,
> Ivan
>