转自http://rednaxelafx.iteye.com/blog/551715
昨天在用findbugs扫我们的代码时看到了类似这样的bug提示:(嗯……真的方法名忽略吧)
findbugs 写道
Bug: alpha.beta.charlie.Foo.bar(String) calls Thread.sleep() with a lock held
Pattern id: SWL_SLEEP_WITH_LOCK_HELD, type: SWL, category: MT_CORRECTNESS
This method calls Thread.sleep() with a lock held. This may result in very poor performance and scalability, or a deadlock, since other threads may be waiting to acquire the lock. It is a much better idea to call wait() on the lock, which releases the lock and allows other threads to run.
Pattern id: SWL_SLEEP_WITH_LOCK_HELD, type: SWL, category: MT_CORRECTNESS
This method calls Thread.sleep() with a lock held. This may result in very poor performance and scalability, or a deadlock, since other threads may be waiting to acquire the lock. It is a much better idea to call wait() on the lock, which releases the lock and allows other threads to run.
那段代码看起来与下面的Foo.bar()类似:
- public class TestSync {
- public static void main(String[] args) throws Exception {
- Foo foo = new Foo();
- Thread t1 = new Thread(new MyRunnable(foo));
- Thread t2 = new Thread(new MyRunnable(foo));
- t1.start();
- t2.start();
- }
- }
- class MyRunnable implements Runnable {
- private Foo foo;
- public MyRunnable(Foo foo) {
- this.foo = foo;
- }
- public void run() {
- foo.bar("alpha");
- }
- }
- class Foo {
- public void bar(String name) {
- String key = name + "_"; // this will create a new "key" object every time invoked
- synchronized (key) {
- try {
- System.out.println("1: " + name);
- Thread.sleep(1000);
- System.out.println("2: " + name);
- Thread.sleep(5000);
- } catch (InterruptedException e) {
- System.err.println("interrupted");
- }
- }
- }
- }
可以看到输出经常是:
1: alpha 1: alpha 2: alpha 2: alpha
嗯,在synchronized块里用了Thread.sleep(),是很邪恶。正准备看看有没有什么办法去修一下,突然发现更糟糕的问题是那synchronized块完全是废的:每次Foo.bar()被调用的时候,“key”都是一个新生成的String对象;于是锁住的根本不是同一个对象,实际上没达到锁的目的。
如果把“+”去掉,并且确保输入的参数是指向同一个String对象(Java里字符串字面量由VM保证会被intern),再看程序的行为就很不同了:
- public class TestSync {
- public static void main(String[] args) throws Exception {
- Foo foo = new Foo();
- Thread t1 = new Thread(new MyRunnable(foo));
- Thread t2 = new Thread(new MyRunnable(foo));
- t1.start();
- t2.start();
- }
- }
- class MyRunnable implements Runnable {
- private Foo foo;
- public MyRunnable(Foo foo) {
- this.foo = foo;
- }
- public void run() {
- foo.bar("alpha");
- }
- }
- class Foo {
- public void bar(String name) {
- String key = name; // notice this doesn't create a new object
- synchronized (key) {
- try {
- System.out.println("1: " + name);
- Thread.sleep(1000);
- System.out.println("2: " + name);
- Thread.sleep(5000);
- } catch (InterruptedException e) {
- System.err.println("interrupted");
- }
- }
- }
- }
输出总是:
1: alpha 2: alpha 1: alpha 2: alpha
然而String这东西,普遍来说在运行时即便equals也无法保证是“同一对象”,所以这代码本身的设计就很有问题,不光是在Thread.sleep()上了。
诶,生产力就消磨在起JBoss,看错误log,关JBoss刷Maven,改代码,刷Maven来build然后再起JBoss……而且遗留下来的问题各种诡异。findbugs默认抓的虫也还不够多 =_=|||| =o) TvT||||