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