logging fixes for last pre-ZBB JDK7-TL snapshot (6977677, 7016208, 7041595)

Mandy Chung mandy.chung at oracle.com
Mon May 16 20:13:51 PDT 2011


  On 5/16/11 7:00 PM, David Holmes wrote:
> Dan,
>
> I'm sorry this didn't come through in time ...
>
> Mandy Chung said the following on 05/17/11 05:22:
>> Logger.java
>>    Looks good.   The removal of the synchronization required by 
>> Logger.getLogger fixes the deadlock issue.
>
> But this method now has a race condition:
>
>  372     // Synchronization is not required here. All synchronization for
>  373     // adding a new Logger object is handled by 
> LogManager.addLogger().
>  374     public static Logger getLogger(String name, String 
> resourceBundleName) {
>  375         LogManager manager = LogManager.getLogManager();
>  376         Logger result = manager.demandLogger(name);
>  377         if (result.resourceBundleName == null) {
>  378             // Note: we may get a MissingResourceException here.
>  379             result.setupResourceInfo(resourceBundleName);
>  380         } else if 
> (!result.resourceBundleName.equals(resourceBundleName)) {
>  381             throw new 
> IllegalArgumentException(result.resourceBundleName +
>  382                                 " != " + resourceBundleName);
>  383         }
>  384         return result;
>  385     }
>
> Two threads calling this method with the same name but different 
> resource bundle names can both see null at line 377 and install their 
> different resource bundles. That section needs to be atomic.
>

My bad - missed this race condition.

The setupResourceInfo method synchronizes on this Logger instance.  The 
resourceBundleName should only be set up to once or same resource bundle 
name.  I think it should check if this.resourceBundleName is null or 
same as the given resourceBundleName; if not, throw IAE.   The 
if-then-else in L377-383 above is no longer needed and can simply call 
result.setupResourceInfo(resourceBundleName).

Mandy



More information about the serviceability-dev mailing list