java-design-patterns
java-design-patterns copied to clipboard
Why are we using synchronized twice in com.iluwatar.singleton.ThreadSafeLazyLoadedIvoryTower?
`/**
- The instance doesn't get created until the method is called for the first time. */ public static synchronized ThreadSafeLazyLoadedIvoryTower getInstance() { if (instance == null) { synchronized (ThreadSafeLazyLoadedIvoryTower.class) { if (instance == null) { instance = new ThreadSafeLazyLoadedIvoryTower(); } } } return instance; }`
Shouldn't we remove synchronized from method signature? I didn't find any explaination for this. Usually for lazy initialization we have synchronized only inside the method right? What pros/cons are there for using synchronized in method signature? Code reference
It's called Double-Checked Locking. REFER Hope your doubt is resolved after reading the above article!
Hi @bharathkalyans thank you for reference.
My doubt is actually if we already have a volatile
instance, any thread will see the latest updated value of that instance. Why do we need to wrap the first null check in a synchronized block(via the method signature).
Please REFER to this article's second section for a better understanding of why volatile must be used!!
I understand why volatile
is used here. What I didn't understand is why we need synchronized in this method signature? public static synchronized ThreadSafeLazyLoadedIvoryTower getInstance()
, given that instance
is already volatile
, why do we need to take a lock here?
You have a good point there @hmittal657 The implementation does look a bit confusing since it mixes synchronized
method with some elements from the double-checked locking. I think we should reduce it to something like this to improve clarity:
public static synchronized ThreadSafeLazyLoadedIvoryTower getInstance() {
if (instance == null) {
instance = new ThreadSafeLazyLoadedIvoryTower();
}
return instance;
}
Note that we are trying to demonstrate different ways to implement the singleton. At the moment we have
- IvoryTower - eagerly initialized thread safe singleton
- ThreadSafeLazyLoadedIvoryTower - basic lazy loaded singleton using
synchronized
- ThreadSafeDoubleCheckedLocking - lazy loaded singleton using double-checked locking
- InitializingOnDemandHolderIdiom - lazy loaded singleton using inner class
- EnumIvoryTower - lazy loaded singleton using enum
I'm also not happy with the current level of documentation we have. There is an open issue to improve it.
@iluwatar Do you want me to work on this?
public static synchronized ThreadSafeLazyLoadedIvoryTower getInstance() {
if (instance == null) {
instance = new ThreadSafeLazyLoadedIvoryTower();
}
return instance;
}
I think it should be enough to remove synchronized
from the method signature. We still would have double check locking and avoid expensive locking in the method calls
@hmittal657 that case is already demonstrated in ThreadSafeDoubleCheckedLocking. We don't want to duplicate it.
@bharathkalyans I'll assign you for this