RFR: 8238566: java.security.Provider$Service.supportsParameter() is racy

[Xuelei Fan]

Looks fine to me.

Xuelei

On 3/11/2020 1:31 PM, Valerie Peng wrote:
> 
> Anyone can help reviewing this?
> 
> I addressed this by applying the double-checked-locking pattern for lazy 
> initialization of instance fields.
> 
> Existing code already have most of the code structured for the pattern 
> but misses the second check.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8238566
> 
> Webrev: http://cr.openjdk.java.net/~valeriep/8238566/webrev.00/
> 
> Thanks,
> 
> Valerie
> 
> On 2/5/2020 12:49 PM, Arthur Eubanks wrote:
>> Webrev: http://cr.openjdk.java.net/~aeubanks/8238566/webrev.00/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8238566
>>
>> Found by TSAN, java.security.Provider$Service.supportsParameter() is 
>> racy. We haven't observed any actual bad behavior, but reasoning 
>> through it seems like bad behavior is possible.
>>
>> http://hg.openjdk.java.net/jdk/jdk/file/62b5bfef8d61/src/java.base/share/classes/java/security/Provider.java#l1927 
>>
>>
>> The synchronized block seems to not have any effect on correctness.
>>
>> Example race:
>> T1 in hasKeyAttributes() writes true to hasKeyAttributes and fills out 
>> supportedFormats/supportedClasses.
>>
>> T2 in hasKeyAttributes() racily reads hasKeyAttributes as true, but 
>> then in supportsKeyFormat() racily reads supportedFormats as null. It 
>> can then improperly return false from supportsParameter().
>> There is no synchronization between T1 and T2 because T2 never does 
>> any synchronization, so T2 can read what T1 writes in any order.
>>
>> Fix is to make all of hasKeyAttributes() synchronized.