confused by synchronized usage

Question

I see the netflix code, in class DynamicPropertyFactory,there's a method like

public static DynamicPropertyFactory getInstance() {
    if (config == null) {
        synchronized (ConfigurationManager.class) {
            if (config == null) {
                AbstractConfiguration configFromManager = ConfigurationManager.getConfigInstance();
                if (configFromManager != null) {
                    initWithConfigurationSource(configFromManager);
                    initializedWithDefaultConfig = !ConfigurationManager.isConfigurationInstalled();
                    logger.info("DynamicPropertyFactory is initialized with configuration sources: " + configFromManager);
                }
            }
        }
    }
    return instance;
}

I'm confused on this method that why using synchronized (ConfigurationManager.class), key word synchronized was used on another class ConfigurationManager.class. From my view, synchronized is used in its current class. So, someone can help explain this simplely?

Yes, that's class level synchronization. Please note, the DynamicPropertyFactory is a class method. — Henry, Dec 19 '16 at 03:30
but it's synchronized (ConfigurationManager.class) not synchronized (DynamicPropertyFactory.class),so.. — lawrence, Dec 19 '16 at 03:33
They only want to initialize class variable config once. During the initialization class method getConfigInstance in class ConfigurationManager is invoked, that's the reason they use class level synchronization (class level lock) — Henry, Dec 21 '16 at 05:07

score 0 · Answer 1 · answered Dec 19 '16 at 03:47

0

Probably because they expect other classes to access ConfigurationManager on some other thread.

answered Dec 19 '16 at 03:47

Karlo Serrano

71
2
8

I guess if a thread invokes method getInstance, and other thread that want to access ConfigurationManager's class or instance method will be blocked, right? – lawrence Dec 19 '16 at 05:44
yes. Although this style seems wierd to me. is this out of curiousity? or are you trying to solve something? – Karlo Serrano Dec 19 '16 at 05:58

score 0 · Answer 2 · edited May 23 '17 at 12:24

Above code was not well written.

DynamicPropertyFactory should not worry about incorporating synchronized checks to get ConfigurationManager instance.

Ideally ConfigurationManager.getConfigInstance() should have proper checks and synchronized (ConfigurationManager.class) check should be moved to ConfigurationManager.

Callers of ConfigurationManager should simply get instance with getConfigInstance() API without any synchronized checks.

Have a look at below SE question to implement Singleton object in right way:

Why is volatile used in this example of double checked locking

confused by synchronized usage

2 Answers2