RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently
Weijun Wang
weijun.wang at oracle.com
Tue May 26 13:45:28 UTC 2015
But here it's not a real singleton (not final), the refresh() method can
reassign an instance to it.
--Max
On 5/26/2015 8:51 PM, Sean Mullan wrote:
> I would use the Initialization-on-demand holder idiom [1] instead, ex:
>
> private static class SingletonHolder {
> private static final Config singleton = new Config();
> }
>
> public static Config getInstance() throws KrbException {
> return SingletonHolder.singleton;
> }
>
> This way you avoid synchronized altogether.
>
> --Sean
>
> [1] http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom
>
>
> On 05/26/2015 12:50 AM, Xuelei Fan wrote:
>> I do not like class level synchronization because it may not work as
>> expected, especially if the synchronization can be used by other codes.
>> However, your update does not change this behavior. The fix looks fine
>> to me. Please go ahead if you don't want to use object level
>> synchronization.
>>
>> Xuelei
>>
>> On 5/26/2015 10:40 AM, Weijun Wang wrote:
>>>
>>>
>>> On 5/26/2015 9:22 AM, Xuelei Fan wrote:
>>>> On 5/26/2015 9:06 AM, Weijun Wang wrote:
>>>>> On 5/26/2015 7:59 AM, Xuelei Fan wrote:
>>>>>> synchronized on class looks a little bit unsafe to me.
>>>>>
>>>>> Why? Isn't it the same as making a static method synchronized? [1]
>>>>>
>>>> Other code may be also able to lock on class.
>>>>
>>>> Code 1:
>>>> lock on MyClass.class
>>>>
>>>> Code 2:
>>>> lock on MyClass.class
>>>>
>>>> Code 1 and 2 know nothing about each other. The behavior may be weird.
>>>> I don't think it is a good practice.
>>>
>>> So you are not a fan of all synchronized methods? :-)
>>>
>>> I thought it's quite common to use synchronized static methods to
>>> prevent simultaneous access to a static field. While a method in another
>>> class can lock on thisClass.class (in this case the class is internal),
>>> I don't see why it wants to do this. This is not casual, it's just evil.
>>>
>>>>
>>>>>> As singleton is
>>>>>> a static variable, creating the instance during initialization looks
>>>>>> safer.
>>>>>>
>>>>>> - private static Config singleton = null;
>>>>>> + private static Config singleton = new Config();
>>>>>
>>>>> This line might throw an exception. Also, do you intend to make
>>>>> getInstance() simply returning singleton? This means if the first call
>>>>> to new Config() throws an exception, getInstance() will not try to
>>>>> reconstruct it. This might not be common in production, but I don't
>>>>> want
>>>>> to make any behavior change.
>>>>>
>>>> I see you point. Maybe, you can lock on a object.
>>>
>>> A private static final Object? Yes I can, but is it worth doing?
>>>
>>> Thanks
>>> Max
>>>
>>>>
>>>> Xuelei
>>>>
>>>>> --Max
>>>>>
>>>>> [1]
>>>>> https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.4.3.6
>>>>>
>>>>>
>>>>>>
>>>>>> Xuelei
>>>>>>
>>>>>> On 5/25/2015 10:16 PM, Weijun Wang wrote:
>>>>>>> Hi All
>>>>>>>
>>>>>>> Please review a code change at
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~weijun/8080911/webrev.00/
>>>>>>>
>>>>>>> I've limit the synchronized block to Config creation only and
>>>>>>> therefore
>>>>>>> won't deadlock with EType's class initialization.
>>>>>>>
>>>>>>> Noreg-hard. The EType call is at class initialization and only run
>>>>>>> once
>>>>>>> in a VM session, which is extremely difficult to catch.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Max
>>>>>>
>>>>
>>
More information about the security-dev
mailing list