Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Daniel, On 8/30/2013 4:50 AM, Daniel Fuchs wrote: Hi, Please find below an updated patch for solution (c) I'm fine with (c) that is a reasonable fix for this bug. The change looks fine with me. Mandy best regards, -- dani

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Ship it! :) Thanks, David On 30/08/2013 9:50 PM, Daniel Fuchs wrote: Hi, Please find below an updated patch for solution (c) best regards, -- daniel >>> It seems we have 5 choices: >>> >>> a. Not fixing >>> b. Using regular

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues CC: core-libs-dev@openjdk.java.net Hi, Please find below an updated patch for solution (c) <http://cr.openjdk.java.net/~dfuchs/webrev_6823527/webrev.02/> best regards, -- daniel

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

> I'm not sure I'd want to attempt that. Modifications in logging code > have a tendency to come back and bite you ;-(... I understand. Maybe a future RFE. I'm happy with your changes then. > isLoggable() is not synchronized and no longer calls synchronized > methods since we're now using vola

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

rds, -- daniel Jason > Date: Fri, 30 Aug 2013 13:50:56 +0200 > From: daniel.fu...@oracle.com > To: david.hol...@oracle.com > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues > CC: core-libs-dev@openjdk.java.net > > Hi, > > Please f

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

racle.com > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety > issues > CC: core-libs-dev@openjdk.java.net > > Hi, > > Please find below an updated patch for solution (c) > > <http://cr.openjdk.java.net/~dfuchs/webrev_6823527/webrev.02/> > > best regards, > > -- daniel >

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Hi, Please find below an updated patch for solution (c) best regards, -- daniel >>> It seems we have 5 choices: >>> >>> a. Not fixing >>> b. Using regular 'synchronized' pattern [1] >>> c. Using volatiles, synchronize setters only

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 30/08/2013 6:38 PM, Daniel Fuchs wrote: On 8/30/13 9:21 AM, David Holmes wrote: StreamHandler.java writer doesn't need to be volatile - AFAICS it is only accessed within synchronized methods. Ah - no - it's called in isLoggable() actually. The whole point of using volatiles instead of regu

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/30/13 9:21 AM, David Holmes wrote: Hi Daniel, I'm fine with the approach in (c), just wanted to highlight the performance considerations (volatile and synchronized need not be equivalent in the uncontended case - biased-locking can mean there are no barriers emitted.) A few specific commen

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Hi Daniel, I'm fine with the approach in (c), just wanted to highlight the performance considerations (volatile and synchronized need not be equivalent in the uncontended case - biased-locking can mean there are no barriers emitted.) A few specific comments on [2]: Handler.java: reportErro

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/30/13 3:27 AM, David Holmes wrote: Hi Daniel, As usual with these kinds of changes we start by trying to complete an incomplete/incorrect synchronization pattern and get diverted into changing the synchronization pattern. :) The latter of course has more risk than the former. Making ev

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Hi Daniel, As usual with these kinds of changes we start by trying to complete an incomplete/incorrect synchronization pattern and get diverted into changing the synchronization pattern. :) The latter of course has more risk than the former. Making everything synchronized has the benefit tha

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/29/13 9:45 PM, Mandy Chung wrote: On 8/29/13 11:04 AM, Daniel Fuchs wrote: On 8/29/13 7:58 PM, Mandy Chung wrote: On 8/29/13 9:46 AM, Daniel Fuchs wrote: Here is the new webrev implementing Jason's suggestion. Compared to previous webrev only Handler.java & StreamHandler.java have changed

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/29/13 11:04 AM, Daniel Fuchs wrote: On 8/29/13 7:58 PM, Mandy Chung wrote: On 8/29/13 9:46 AM, Daniel Fuchs wrote: Here is the new webrev implementing Jason's suggestion. Compared to previous webrev only Handler.java & StreamHandler.java have changed.

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Date: Thu, 29 Aug 2013 18:46:53 +0200 > From: daniel.fu...@oracle.com > To: core-libs-dev@openjdk.java.net > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues > > On 8/29/13 5:38 PM, Daniel Fuchs wrote: > > Hi Jason, > > > > Yes -

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Looks good. Shouldn't 'boolean sealed' be declared volatile? Jason > Date: Thu, 29 Aug 2013 18:46:53 +0200 > From: daniel.fu...@oracle.com > To: core-libs-dev@openjdk.java.net > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety >

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/29/13 7:58 PM, Mandy Chung wrote: On 8/29/13 9:46 AM, Daniel Fuchs wrote: Here is the new webrev implementing Jason's suggestion. Compared to previous webrev only Handler.java & StreamHandler.java have changed. Looks good.

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/29/13 9:46 AM, Daniel Fuchs wrote: Here is the new webrev implementing Jason's suggestion. Compared to previous webrev only Handler.java & StreamHandler.java have changed. Looks good. I also agree that making those fields to

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

On 8/29/13 5:38 PM, Daniel Fuchs wrote: Hi Jason, Yes - that makes sense. I think we want the setter to use the same lock than e.g. publish because we don't want the level (or filter or encoding or whatever) to be changed while publish is executing. However I see no harm in allowing other threa

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Hi Jason, Yes - that makes sense. I think we want the setter to use the same lock than e.g. publish because we don't want the level (or filter or encoding or whatever) to be changed while publish is executing. However I see no harm in allowing other threads to read the variables values while set

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

> Yes - this is a possibility I envisaged too. > But would that be better? > > I didn't want to remove any synchronized blocks because I'm > really not sure of what consequences it might have. > getLevel()/setLevel() are already synchronized on Handler. > It is my belief that most

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

ly - when it's clear that it's the better solution. best regards, -- daniel > Date: Thu, 29 Aug 2013 21:33:07 +1000 > From: david.hol...@oracle.com > To: daniel.fu...@oracle.com > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues > CC:

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

you would only block threads that are actually publishing information of interest. Jason > Date: Thu, 29 Aug 2013 21:33:07 +1000 > From: david.hol...@oracle.com > To: daniel.fu...@oracle.com > Subject: Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety > is

Re: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

Hi Daniel, On the surface these changes seem reasonable - the code looks the way it should have been written in the first place. The risk with these kinds of changes are always performance and deadlocks. :) I would not be surprised, given this is logging, that somewhere there is a path that