<i18n dev> testcase failure java/util/Locale/Bug6989440.java

[Yoshito Umaoka]

Thanks of all of you.
This problem was introduced by the contribution from ICU project, and 
I'm responsible to the specific code.

-Yoshito

On 10/6/2011 5:59 PM, Naoto Sato wrote:
> Hi David,
>
> Thank you for the quick look and the fix!
>
> Naoto
>
> On 10/6/11 10:09 AM, David Holmes wrote:
>> Hi Chris,
>>
>> Thanks. Here's the bug:
>>
>> private List<Locale> getJRELocales() {
>> if (availableJRELocales == null) {
>> synchronized (LocaleServiceProviderPool.class) {
>> if (availableJRELocales == null) {
>> Locale[] allLocales = LocaleData.getAvailableLocales();
>> availableJRELocales = new ArrayList<Locale>(allLocales.length); <<====
>> YIKES we just published an empty ArrayList
>> for (Locale locale : allLocales) {
>> availableJRELocales.add(getLookupLocale(locale));
>> }
>> }
>> }
>> }
>> return availableJRELocales;
>> }
>>
>>
>> availableJRELocales is a static variable shared across all pools. But it
>> is being published before it gets populated. We need to use a temporary
>> for the new ArrayList and only assign to availableJRELocales after it is
>> populated.
>>
>> In addition, as you mentioned, availableJRELocales needs to be volatile
>> for this DCL pattern to be correct.
>>
>> Thanks,
>> David
>>
>> On 6/10/2011 9:02 PM, Chris Hegarty wrote:
>>> I see several exceptions, similar to the following (numbers vary):
>>>
>>> Exception in thread "Thread-23"
>>> java.util.ConcurrentModificationException: Expected: 96, got: 156
>>> at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:819)
>>> at java.util.ArrayList$Itr.next(ArrayList.java:791)
>>> at java.util.AbstractCollection.addAll(AbstractCollection.java:333)
>>> at java.util.HashSet.<init>(HashSet.java:117)
>>> at
>>> sun.util.LocaleServiceProviderPool.getAvailableLocales(LocaleServiceProviderPool.java:206) 
>>>
>>>
>>>
>>> at Interrupter$TestThread.run(Interrupter.java:49)
>>
>>
>>>
>>> There would appear to be 156 JRE Locales ( at least on this system ),
>>> modCount is incremented for each add(), but when the iterator is 
>>> created
>>> ( implicitly during the HastSet.addAll ) it sees a different value for
>>> modCount.
>>>
>>> I think there is a visibility issue here. availableJRELocales is
>>> volatile, but the List reference returned from getJRELocales is not. In
>>> the case where availableJRELocales is not null there will be no memory
>>> barrier to force a HB relationship. Or maybe I've gotten this wrong? 
>>> His
>>> is quite bizarre, or maybe it is just the overly complicated use of
>>> locking/DCL in this class.
>>>
>>> -Chris.
>>>
>>> On 10/ 5/11 07:01 PM, David Holmes wrote:
>>>> This might not be related to the CME problem, but here:
>>>>
>>>> public static LocaleServiceProviderPool getPool(Class<? extends
>>>> LocaleServiceProvider> providerClass) {
>>>> LocaleServiceProviderPool pool = poolOfPools.get(providerClass);
>>>> if (pool == null) {
>>>> LocaleServiceProviderPool newPool =
>>>> new LocaleServiceProviderPool(providerClass);
>>>> pool = poolOfPools.put(providerClass, newPool);
>>>> if (pool == null) {
>>>> pool = newPool;
>>>> }
>>>> }
>>>>
>>>> return pool;
>>>> }
>>>>
>>>> we should probably be using poolOfPools.putIfAbsent(providerClass,
>>>> newPool)
>>>>
>>>> David
>>>>
>>>> On 6/10/2011 3:35 AM, Naoto Sato wrote:
>>>>> I will look into this. Reopened the original CR.
>>>>>
>>>>> Naoto
>>>>>
>>>>> On 10/5/11 9:58 AM, Alan Bateman wrote:
>>>>>> Chris Hegarty wrote:
>>>>>>> Alan, Naoto, David
>>>>>>>
>>>>>>> I filed CR 7098100: java/util/Locale/Bug6989440.java fails
>>>>>>> intermittently.
>>>>>>>
>>>>>>> If you're ok with it please review the patch (below) and I can
>>>>>>> push it
>>>>>>> to the tl repo. Job done!
>>>>>> I assume there is also some underlying issue in the Locale code and
>>>>>> this
>>>>>> might get hidden if we fix the test (I"ve no objection to fixing the
>>>>>> test of course, just thinking that we should study the Locale 
>>>>>> code to
>>>>>> try to identify the deadlock or hang or whatever it is that is 
>>>>>> causing
>>>>>> the threads in this test not to terminate).
>>>>>>
>>>>>> -Alan
>>>>>
>