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

[Daniel D. Daugherty]

On 5/16/2011 1:22 PM, Mandy Chung wrote:
>  Dan,
>
> Great that you knocked down another hard-to-reproduce bug and your 
> simple regression test.  The comments you add are really helpful.

Thanks!


>
> Logger.java
>    Looks good.   The removal of the synchronization required by 
> Logger.getLogger
>    fixes the deadlock issue.

Thanks! I've done a crawl through Logger.java and LogManager.java
and closely examined all uses of "synchronized". I'll be filing a
new bug to track those notes and the resolution of those potential
issues. Of course, that'll be a JDK8 thing...


> LogManager.java
>     Retrying in a while loop is one way to fix weak ref / gc timing 
> issue.

Tom is the one that suggested the loop. I just happened
to write it a bit differently than he would have, but
for today's snapshot I think this is good.


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

This should also do the trick, but I think I would prefer to
go with what I have since it's tested and reviewed by several
folks (most of the comments have been off-list).


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

Yes it probably would fix 7016208, but I would prefer to go
with what I have. Yes, addLogger() returning a boolean was
a bad design choice way back when.

We'll have to look at tweaking the Logging API in JDK8.

Are you okay if I go with what I have (modulo a few editorial
changes)?

Dan




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