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

[Dmitry Samersoff]

David,

I'd also stopped my eyes on this line but actually this race changes 
nothing in Logger behavior - setupResourceInfo just replace previous 
assignment.

     As far as I understand Logger object could be collected out between 
two getLogger() calls so nobody can be sure that assigned 
resourceBundleName will be returned.

So no reason to worry about this race right now.

-Dmitry


On 2011-05-17 06:00, 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.
>
> David
> -----
>
>>
>> LogManager.java
>> Retrying in a while loop is one way to fix weak ref / gc timing issue.
>> What about if you refactor the addLogger method to add a new private
>> private method equivalent to addLogger (say add(Logger)) that will return
>> logger if added successfully or null if not.
>>
>> The addLogger method will return add(logger) != logger. The
>> synchronization
>> will be done in the new add method.
>>
>> Would this fix 7016208? It seems to me that addLogger returning boolean
>> is the cause for this bug.
>>
>> Mandy
>>
>> On 05/15/11 11:40, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> The final pre-ZBB JDK7-TL snapshot is happening @ 1700 PT on
>>> Monday, May 14, 2011. This snapshot is targeted for JDK7-B143
>>> or JDK7-B144 (I'm not sure which). Yes, we're still trying to
>>> figure out how to merge our JDK & HotSpot Express processes
>>> with Oracle processes. Please be patient while we iron out the
>>> wrinkles...
>>>
>>> I have three bug fixes for the java.util.logging area:
>>>
>>> 6977677 3/2 Deadlock on logging subsystem initialization
>>> 7016208 4/3 null sometimes returned by java.util.logging.Logger.
>>> getLogger(String name) in -server -Xcomp
>>> 7041595 4/4 add lost test for 6487638
>>>
>>> 6977677 is a deadlock between java.util.logging.Logger.getLogger()
>>> and LogManager.<clinit> via a PlatformLogger. This fix involves:
>>>
>>> src/share/classes/java/util/logging/Logger.java
>>> test/java/util/logging/LoggingDeadlock4.java
>>>
>>> Mandy, I would like your review of the above bug fix.
>>>
>>> 7016208 is an unexpected null return from Logger.getLogger() due to
>>> the lack of a strong reference. This fix involves:
>>>
>>> src/share/classes/java/util/logging/LogManager.java
>>>
>>> Tom R., I would like your review of the above bug fix.
>>>
>>> 7041595 is just pushing a Logging deadlock test that got lost a
>>> long time ago. This fix involves:
>>>
>>> test/java/util/logging/LoggingDeadlock3.java
>>> test/java/util/logging/LoggingDeadlock3.props
>>>
>>> Because I'm fixing a deadlock in 6977677, I wanted to make sure
>>> that this Logging deadlock test was back in the mix.
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/logging-batch-20110515-webrev/0/
>>>
>>> The comments that I added to the code changes should make the
>>> reason(s) for the code changes pretty self explanatory.
>>>
>>> These changes have been run through JPRT and pass the "jdk_util"
>>> tests on all platforms. I have also run SDK/JDK logging tests
>>> and the VM/NSK logging tests on the following configs:
>>>
>>> Solaris X86 * {Client VM, Server VM} product * {-Xmixed, -Xcomp}
>>> WinXP * {Client VM, Server VM} product * {-Xmixed, -Xcomp}
>>>
>>> Here is the summary from Solaris X86:
>>>
>>> Summary of Test Results (8 result dirs)
>>> =========================================
>>> all executed: 2264 all passed: 2264 all ignored: 0 all failed: 0
>>> time: 0 hour(s) 17 minute(s)
>>>
>>> Here is the partial summary (6 of 8) from WinXP:
>>>
>>> Summary of Test Results (6 result dirs)
>>> =========================================
>>> all executed: 1160 all passed: 1160 all ignored: 0 all failed: 0
>>> time: 1 hour(s) 0 minute(s)
>>>
>>> The last two VM/NSK test runs are still going; yes, WinXP is slower than
>>> Solaris X86...
>>>
>>> Thanks, in advance, for any reviews.
>>>
>>> Dan
>>>
>>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...