I'm jumping into the middle of a conversation that I haven't followed
closely, so this could be inapplicable, but it seems to me that there is
an option #4, namely Avalon-style explicit provisioning of a Logger when
the DatabaseManager is constructed.
Just a thought...
Gary
On Tue, 19 Nov 2002, at 20:00 [-0000], Kevin Smith ([EMAIL PROTECTED]:
> Here's the threading issue I see:
>
> public class DatabaseManager
> {
> private transient Logger logger;
>
> public Database getDatabase() {
> Logger logger = getLogger();
> logger.debug("Getting database");
> // do something here and return database
> }
> protected Logger getLogger() {
> if(logger == null)
> logger = LogFactory.getLogger(getClass());
>
> }
> }
>
> If two threads call getDatabase() at the same time, there is a chance that you
> could wind up allocating two loggers when only one is needed. Since the
> logger is thread-safe, we need to make getting a reference to a logger
> thread-safe, too.
>
> There are three ways of doing it:
>
> 1) Synchronize getLogger() - This introduces synchronization overhead every
> time you need a logger.
>
> 2) Synchronize all methods that call getLogger() - Ugly.
>
> 3) Make the logger a static variable - This moves the initialization of the
> logger to when the class is loaded and thus, removes any threading issues.
> The penalty for such a refactoring is having one line of duplicate code in
> each class:
>
> private static Logger logger = LogFactory.getLogger(Database.class.getName());
>
> That's why I was suggesting using #3.
>
> --Kevin
> On Tuesday 19 November 2002 07:52 pm, Kevin Ross wrote:
> > >Obtaining loggers might be a bad example. All I'm saying that in my
> >
> > experience, this kind of resource initialization scheme is better off
> > being thread safe from >the start. It eliminates a whole family of bugs
> > that you need to worry about down the road.
> >
> > Agreed, but in the case of logging, I believe you should err on the side
> > of performance for the greater good, the 80%+ case. Bothering with
> > paranoid initialization (synchronization) makes everyone instance pay
> > the price.
> >
> > Just got your next message. I'll say it one more time, WHAT thread
> > safety issues???? The logger is responsible for thread safety. I don't
> > implement synchronization when initializing or using a Logger, because
> > it is unnecessary for the client to do so. I haven't for years now, and
> > I haven't had one problem in organizations with traffic from a few
> > thousand hits a day, to a few million.
> >
> > Duplication of code is one of the fundamentally wrong things to do in OO
> > programming, and I agree, therefore I stay away from it.
> >
> > -Kevin Ross
> >
> > Kevin A. Smith wrote:
> > > I just thought I'd bring it up. Personally, I err on the side of
> > > paranoia when I can. I don't enjoy debugging threading issues.
> > >
> > > If the LogFactory is recycling logger instances, then at most you pay
> > > the price of one extra method call. BUT, if the LogFactory is creating
> > > a new logger per call, then you're (potentially) paying the price of
> > > creating an object that won't be used and an extra method call.
> > >
> > > Obtaining loggers might be a bad example. All I'm saying that in my
> > > experience, this kind of resource initialization scheme is better off
> > > being thread safe from the start. It eliminates a whole family of bugs
> > > that you need to worry about down the road.
> > >
> > > --Kevin
> > >
> > > -----Original Message-----
> > > From: Kevin Ross [mailto:[EMAIL PROTECTED]
> > > Sent: Tuesday, November 19, 2002 2:30 PM
> > > To: [EMAIL PROTECTED]
> > > Subject: Re: LogFactory.getLogger()
> > >
> > > Threading issues?? I knew someone would bring that up. Thread
> > > safetyness is the responsibility, gladly accepted by a logger
> > > instance, not on the classes using it. There is NO reason why we
> > > need paranoid checking here to only get one instance, since it is
> > > the responsibility of the logging implementation to even provide
> > > the instance in the first place. Do you really think you'll get
> > > different instances? Probably not, worst case scenario, you call
> > > Logger.getLogger() twice and initialize your instance twice in a
> > > race condition. Since it is of zero consequence either way, there
> > > is no reason to do it differently. If you are particularly
> > > worried, just do:
> > >
> > > private transient Logger logger = LogFactory.getLogger(getClass());
> > >
> > > You CANNOT use MyClass.class and have MySubClass.class show up in
> > > the log statements, since this is static. You must use dynamic
> > > initialization.
> > >
> > > -Kevin
> > >
> > > Kevin A. Smith wrote:
> > >> That doesn't look thread-safe to me. You'd need to somehow
> > >> synchronize access to getLogger() to eliminate a race-condition
> > >> on the if(logger == null) test.
> > >>
> > >> While its a bit more typing, I prefer:
> > >>
> > >> private static Logger logger = LogFactory.getLogger(MyClass.class);
> > >>
> > >> or
> > >>
> > >> private static Logger logger =
> > >> LogFactory.getLogger(MyClass.class.getName());
> > >>
> > >> No threading issues and is pretty easy to read and understand.
> > >>
> > >> --Kevin
> > >>
> > >> -----Original Message-----
> > >> From: Kevin Ross [mailto:[EMAIL PROTECTED]
> > >> Sent: Tuesday, November 19, 2002 2:18 PM
> > >> To: Vladimir R. Bossicard; [EMAIL PROTECTED]
> > >> Subject: Re: LogFactory.getLogger()
> > >>
> > >> As I mentioned in my previous message, this is a poor way to
> > >> obtain a logger, especially if you have deep inheritance
> > >> heirarchies. Since I'm a big fan of OO programming, and my
> > >> heirarchies are especially deep, I find the static way of
> > >> obtaining a logger invalid.
> > >>
> > >> Try this:
> > >>
> > >> public class AbstractLoggable{
> > >>
> > >> private static transient Logger logger;
> > >>
> > >> protected Logger getLogger(){
> > >>
> > >> if( logger == null ){
> > >>
> > >> logger = Logger.getLogger(getClass());
> > >> }
> > >>
> > >> return logger;
> > >> }
> > >> }
> > >>
> > >> whallah! efficient OO programming with an accessor for
> > >> subclasses and the proper class name to go with it!
> > >>
> > >> -Kevin Ross
> > >>
> > >> Vladimir R. Bossicard wrote:
> > >>>>Logger.getLogger(getClass()) at least, whether or not you use the
> > >>>> string name is of no consequence.
> > >>>
> > >>>have you ever tried to call a non-static method within a static
> > >>> reference?
> > >>>
> > >>>if you get this working:
> > >>>
> > >>> private static Log log = LogFactory.getLog(getClass());
> > >>>
> > >>>I give you a high-five.
> > >>>
> > >>>-Vladimir
> > >>>
> > >>>--
> > >>>Vladimir R. Bossicard
> > >>>www.bossicard.com
>
>
>
