RFR JDK-8155005: java.lang.reflect.Module.WeakSet is not thread-safe

[Peter Levart]

Hi Alan,

On 04/25/2016 02:58 PM, Alan Bateman wrote:
> On 25/04/2016 12:13, Peter Levart wrote:
>> Hi Alan,
>>
>> I created an issue for this:
>>
>>     JDK-8155005: java.lang.reflect.Module.WeakSet is not thread-safe
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8155005
>>
>>
>> I did what you suggested, renamed type parameters to <K1, K2, V>, 
>> split long line in Module and modified the test so that it now waits 
>> for entry to be expunged for up to 5 seconds when it is expected to 
>> be expunged but returns as soon as it detects that the entry is gone 
>> (usually in 1st 100 ms). It still waits just 500 ms when the entry is 
>> expected to remain in the map. False negatives should be eliminated 
>> this way and the test still doesn't waist plenty of execution time.
>>
>> Here's latest webrev:
>>
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Module.WeakSet.multithreadUnsafe/webrev.04/ 
>>
> I think using <K1, K2, V> makes it a lot more readable - thanks.
>
> On gcAndWaitRemoved then I wonder if it would be more robust to have a 
> variant that polls indefinitely. 5s is a long time but with fastdebug 
> builds, maybe a looping thread or processes left over from a previous 
> test runs, virtual machines, then it might be long enough.

I increased the timeout to 30 seconds. It is not exactly 30 seconds, but 
300 iterations with sleep(100L) + check in each iteration. If the system 
is really overloaded then this loop should stretch automatically:

http://cr.openjdk.java.net/~plevart/jdk9-dev/Module.WeakSet.multithreadUnsafe/webrev.05/

Are there any other jigsaw-specific tests I should be paying close 
attention to? I ran the jdk/test/java/lang/reflect/WeakPairMap 
jdk/test/java/lang/reflect/Module and jdk/test/java/lang/Class tests.

Will I need an official blessing to push this to jdk9/dev on the 
core-libs-dev as we have been discussing this patch only on jigsaw-dev 
for a while?

Regards, Peter


>
>>
>>
>> If this is accepted then I would need to know via which repo this 
>> should be pushed (jdk9/dev or jake).
> It should be okay to push this to jdk9/dev and we'll pull the changes 
> into jake. In general then we will be iterating on the module code for 
> some time and so will be using the jake sandbox for that.
>
> -Alan