Found a multi-threading synchronization problem, my code look like this:
class ASimpleTagClass extends SimpleTagSupport {
static class SingletonHolder {
static SomeClass instance = new SomeClass();
}
static boolean inited = false;
synchronized init() {
if (!inited) {
//… blah blah init codes
}
}
}
Because SimpleTagSupport will create an Instance for each, the ASimpleTagClass’s init() is not thread safe, because e synchronized method will prevent two invocations of the methods on the same object, not two different objects. Here obviously they are different instances, so code inside init() will run concurrently and will cause bug.
An article ” Understand that for instance methods, synchronized locks objects, not methods or code” is exactly talking about this issue:
The synchronized keyword is used as either a method modifier or as a statement inside of a method. This dual usage causes some confusion about exactly what the synchronized keyword does. It is often described in terms of mutual exclusion (mutex) or a critical section. This causes many programmers to incorrectly think that because code is protected by synchronized , it can be accessed by only one thread at a time.
For instance methods, the synchronized keyword does not lock a method or code; it locks objects. Remember that there is only one lock associated with each object.
When synchronized is used as a method modifier, the lock obtained is that for the object on which the method was invoked.When it is used on an object refer-ence, the lock obtained is that for the object referred to. For example, consider the following code:
class Test
{
public synchronized void method1()
{
//…
}
public void method2()
{
synchronized(this) {
//…
}
}
public void method3(SomeObject someObj)
{
//…
synchronized(someObj) {
//…
}
}
}
|
The first two methods, method1 and method2 , are functionally identical regarding the object being locked. (They differ in the amount of code generated and in how they perform.) Both methods are synchronized on this . In other words, a lock is obtained for the object on which the method was invoked. Because both methods are of the class Test , the lock is obtained for an object of class Test . The method3 method is synchronized on the object referenced by someObj .
What exactly does it mean to synchronize on an object? It means that the thread that invoked the method has that object s lock. To hold an object s lock means another thread requesting a lock for the same object through a synchronized method or a synchronized statement cannot obtain that lock until it is released. However, another thread executing the same synchronized method or block on a different instance of the same class can obtain that instance s lock.
Change my code to:
class ASimpleTagClass extends SimpleTagSupport {
static class SingletonHolder {
static SomeClass instance = new SomeClass();
}
static Boolean inited = false;
void init() {
synchronized(inited) {
if (!inited) {
//… blah blah init codes
}
}
}
}
Then the problem should gone.
Lesson learned:
Remember that synchronization locks objects, not methods or code. Simply because a method, or section of code, is declared synchronized does not necessarily mean it is executed by only one thread at a time. This is important if the code in the synchronized method or block alters a mutually shared resource.
Another interesting thing is, if init() in my sample could be a static method, then it will not have thread safe problem. Because:
class MyClass {
…
public synchronized static someMethod() {
…
}
…
}
Is it the equivalent to:
synchronized ( MyClass.class ) {
…
}
from:
The JVM creates a Class object when the class is loaded (When it is used for the first time) the
JVM creates one instance of Class for each class that is loaded, thus a class itself has a moniter
which a thread can gain ownership of.
—-
UPDATED : the above changed code is still buggy!!! And may contain hidden problem:
static Boolean inited = false;
“inited” may reference to some Boolean object which may be used else where! (That’s Java’s nature, it pool some objects for reuse. ) So if there are some where else have similar code, and lock on this object, it may cause dead lock! And such problem may be hard to find out.
Change the code to:
…
synchronized(ASimpleTag.class) {
…
Popularity: 6% [?]
Share This
About