Using Synchronization while Logging

Question

In my application I have written my own Logging utility using Java.Util.Logging

import java.io.IOException;
import java.io.InputStream;
import java.util.Properties;
import java.util.logging.FileHandler;
import java.util.logging.Level;
import java.util.logging.SimpleFormatter;



public class Logger {

    public final static String PROPERTIES_FILE = "Logger.properties";
    private static java.util.logging.Logger logger = null;

    private static void initialize() {
        try {
            logger = java.util.logging.Logger.getLogger(Logger.class.getName());
            FileHandler fh = new FileHandler("D:\\MyLogFile.log", true);
            logger.addHandler(fh);
            logger.setLevel(Level.ALL);
            SimpleFormatter formatter = new SimpleFormatter();
            fh.setFormatter(formatter);
            logger.log(Level.INFO, "Test Message Logged");

        }
        catch (IOException e) {
          System.out.println("Warning: Unable to read properties file " +
                             PROPERTIES_FILE );
        }   
      }

    public static synchronized java.util.logging.Logger getLogger(String name)
    {
        if(logger==null)
        {
        Logger.initialize();
        }
        logger.getLogger(name);
        return logger;
    }


}

Do I need to use Synchronization for getLogger method? Please give your comments. (This code is running in Multi-threaded environment)

ykaganovich · Accepted Answer · 2013-08-08 21:16:43Z

I agree with other commenters that lazy initialization doesn't seem necessary here. The simplest way to initialize the logger variable is in a static initializer, which is guaranteed to only execute once at class loading time:

public class Logger {

    public final static String PROPERTIES_FILE = "Logger.properties";
    private static java.util.logging.Logger logger = null;

    private static void initialize() {
        try {
            logger = java.util.logging.Logger.getLogger(Logger.class.getName());
            FileHandler fh = new FileHandler("D:\\MyLogFile.log", true);
            logger.addHandler(fh);
            logger.setLevel(Level.ALL);
            SimpleFormatter formatter = new SimpleFormatter();
            fh.setFormatter(formatter);
            logger.log(Level.INFO, "Test Message Logged");

        }
        catch (IOException e) {
          System.out.println("Warning: Unable to read properties file " +
                             PROPERTIES_FILE );
        }   
    }

    static {
        initialize();
    }

    public static java.util.logging.Logger getLogger(String name)
    {
        logger.getLogger(name);
        return logger;
    }


}

However, You can avoid most synchronization with double-checked locking.

public class Logger {

    // note: volatile is required
    private volatile static java.util.logging.Logger logger = null;

    //... 

    public static java.util.logging.Logger getLogger(String name)
    {
        if(logger==null)
        {
           synchronized(Logger.class) 
           {
              if(logger == null)
                Logger.initialize();
              }
           }
        }
        logger.getLogger(name);
        return logger;
    }
}

In fact, in your case I think you can avoid synchronization altogether if you rewrite your initialize function so that it completely configures the logger in an local variable prior to assigning it to the (volatile) class variable:

private volatile static java.util.logging.Logger logger = null;
private static void initialize() {
    try {
        Logger logger = java.util.logging.Logger.getLogger(Logger.class.getName());
        FileHandler fh = new FileHandler("D:\\MyLogFile.log", true);
        logger.addHandler(fh);
        logger.setLevel(Level.ALL);
        SimpleFormatter formatter = new SimpleFormatter();
        fh.setFormatter(formatter);
        logger.log(Level.INFO, "Test Message Logged");

        Logger.logger = logger;
    }
    catch (IOException e) {
      System.out.println("Warning: Unable to read properties file " +
                         PROPERTIES_FILE );
    }   

    public static java.util.logging.Logger getLogger(String name)
    {
        if(logger==null)
        {
        Logger.initialize();
        }
        logger.getLogger(name);
        return logger;
    }
}

This has a potential to have initialize() execute several times, but I don't think you care, so long that every getLogger invocation will have a valid logger instance, even if that instance varies.

As pointed out in the double-checked locking article ykaganovich cited, the example he gave of double-checked logging only works in Java 5 or later. Prior to Java 5 there was no way to avoid the synchronized keyword because of the way the JVM could distribute threads across processors (and their local memory caches). In general I agree that static initialization of a singleton is best where possible.

Tomasz Nurkiewicz · Accepted Answer · 2012-01-18 18:38:42Z

0

Yes, you need synchronized there. First to avoid calling initialize() several times and secondly to make logger change visible to other threads.

This begs a question: why can't you eagerly initialize() logger and avoid synchronization altogether?

answered Jan 18, 2012 at 18:38

Tomasz Nurkiewicz

342k72 gold badges713 silver badges680 bronze badges

4 Comments

Sunny Gupta Over a year ago

You mean to say i can call initialize from the static block.

Tomasz Nurkiewicz Over a year ago

@sunnygupta: If this is a question, then yes :-). Calling it inside static initialization block of Logger means it will be called once the Logger class is loaded for the first time. Probably when some other class uses it (e.g. by referencing in a static field).

Sunny Gupta Over a year ago

I need not call initialize many times as I am returning the same object everytime. kind of singleton implementation

Tomasz Nurkiewicz Over a year ago

@sunnygupta: using static initialization block guarantees that (put aside different class-loaders)

Jeremiah Orr · Accepted Answer · 2012-01-18 18:47:11Z

0

I wouldn't recommend it, because you'll be incurring the overhead of synchronization on every call to getLogger() when you only need it once for initialization. I'd do this instead:

private static boolean initialized = false;

private static synchronized void initialize() {
    try {
       if(!initialized)
       {            
        // Do initialization
        initialized = true;
       }

    }
    catch (IOException e) {
      System.out.println("Warning: Unable to read properties file " +
                         PROPERTIES_FILE );
    }   
  }

public static java.util.logging.Logger getLogger(String name)
{
    if(logger==null)
    {
    Logger.initialize();
    }
    logger.getLogger(name);
    return logger;
}

This way, if a second thread comes in while the first thread is still in initialize(), the second thread will block because of synchronized on initialize(). Once the first thread is done with initialization, the second thread proceeds, sees that initialized is true, doesn't redo the initialization, and moves on.

answered Jan 18, 2012 at 18:47

Jeremiah Orr

2,6301 gold badge20 silver badges26 bronze badges

2 Comments

ykaganovich Over a year ago

This is essentially double-checked locking, except this code is wrong in a couple of places. a) initialized needs to be volatile. b) checking different conditions !initialized vs logger==null is dangerous. getLogger should also check !initialized

Jeremiah Orr Over a year ago

a) Definitely true. b) Also true, though I'll admit I was trying to do it with the fewest changes to the poster's code. Anyway, I like your static initializing block better.

Collectives™ on Stack Overflow

Using Synchronization while Logging

3 Answers 3

1 Comment

4 Comments

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

4 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related