[hyddd的FindBugs分析记录][M M IS] Inconsistent synchronization
2009-02-15 21:22 hyddd 阅读(5583) 评论(0) 编辑 收藏 举报[M M IS] Inconsistent synchronization [IS2_INCONSISTENT_SYNC]
The fields of this class appear to be accessed inconsistently with respect to synchronization. This bug report indicates that the bug pattern detector judged that
- The class contains a mix of locked and unlocked accesses,
- At least one locked access was performed by one of the class's own methods, and
- The number of unsynchronized field accesses (reads and writes) was no more than one third of all accesses, with writes being weighed twice as high as reads
A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.
You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.
Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held. Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.
先上一段代码:
public static void main(String args[]) throws InterruptedException{
ObjectClass obj = new ObjectClass();
Thread t2 = new ChangeValue(obj);
t2.start();
Thread t1 = new AlwaysRun(obj);
t1.start();
sleep(10000);
t1.stop();
}
}
class AlwaysRun extends Thread{
ObjectClass obj;
public AlwaysRun(ObjectClass obj) {
// TODO Auto-generated constructor stub
this.obj = obj;
}
public void run() {
obj.Loop();
}
}
class ChangeValue extends Thread{
ObjectClass obj;
ChangeValue(ObjectClass obj){
this.obj = obj;
}
public void run() {
System.out.println("Thread2");
ObjClass2 obj2 = obj.getObj();
try {
sleep(1500);
} catch (InterruptedException e) {
System.out.println("Error!");
}
obj2.str = "aaa";
System.out.println("Thread2 Finish!");
}
}
public class ObjectClass extends Thread {
private ObjClass2 obj;
private Object lockTable = new Object();
public ObjectClass() {
// TODO Auto-generated constructor stub
obj = new ObjClass2();
}
public void setObj(ObjClass2 obj){
synchronized (lockTable){
this.obj = obj;
}
}
public ObjClass2 getObj(){
synchronized (lockTable){
return this.obj; //出问题处!!
}
}
public void Loop(){
synchronized (lockTable){
while(true){
System.out.println(obj.str);
}
}
}
}
public class ObjClass2 {
public String str = "ddddddd";
}
看看运行的结果:
Thread2
ddddddd
ddddddd
ddddddd
ddddddd
ddddddd
ddddddd
ddddddd
Thread2 Finish!
aaa
aaa
aaa
aaa
aaa
aaa
....
如果看明白代码,你应该会知道问题出再哪里了,为什么Loop使用了synchronized (lockTable),但是obj.str还是被修改了?!因为getObj()这个函数把obj对象返回了给外面,JAVA里面对象的传递是使用引用传递,如果对象传递到外面并且在做修改obj的时候没有加锁操作,就是引起刚才的问题。所以如果getObj()函数返回的是对象,那么,请返回一个拷贝,而不要直接返回引用。
这里再总结一下值得注意问题:
1.看下面代码:
synchronized (lockTable){
return this.obj;
}
}
另外建议的是,不直接返回this.obj,而是返回一个this.obj的拷贝。这样可以根本上避免出现上面的问题!
2.同理,在setObj(...)的时候,如果传入的是个对象,也建议是存储传入对象的拷贝,而不(this.obj=obj)这样直接赋值。
3.注意对竞争的资源都使用synchronized (lockTable),不要像上面的Demo代码那样,一处用了,一处没有!
作者:hyddd
出处:http://www.cnblogs.com/hyddd/
本文版权归作者所有,欢迎转载,演绎或用于商业目的,但是必须说明本文出处(包含链接)。