RFR 8080911: sun/security/krb5/auto/UseCacheAndStoreKey.java timed out intermittently

[Sean Mullan]

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
>>>>>
>>>
>