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

[Ivan Gerasimov]

On 27.03.2014 18:00, Peter Levart wrote:
> 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?
>
Yeah.
I think that the test will only bring attention if it fails.


> Regards, Peter
>
>>
>> Sincerely yours,
>> Ivan
>>
>