从一篇Blog看两个并发编程错误

发现公司支付宝接入的代码有点神奇，在网上搜索了下，找到原始版本。估计有不少人都是抄那份代码的。

原文在：http://blog.csdn.net/simdanfeg/article/details/9011603    Android支付接入（一）：支付宝

但是代码里有两个明显的并发问题，尽管在Android下可能不会有问题。

下面摘抄一段：

    public class MobileSecurePayer {  
        <strong>Integer lock = 0;  </strong>  
        // 和安全支付服务建立连接  
        private ServiceConnection mAlixPayConnection = new ServiceConnection (){  
            public void onServiceConnected (ComponentName className, IBinder service){  
                //  
                // wake up the binder to continue.  
                // 获得通信通道  
                <strong>synchronized (lock)</strong>{  
                    mAlixPay = IAlixPay.Stub.asInterface (service);  
                    lock.notify ();  
                }  
            }  


            // 实例一个线程来进行支付  
            new Thread (new Runnable (){  
                public void run (){  
                    try{  
                        // wait for the service bind operation to completely  
                        // finished.  
                        // Note: this is important,otherwise the next mAlixPay.Pay()  
                        // will fail.  
                        // 等待安全支付服务绑定操作结束  
                        // 注意：这里很重要，否则mAlixPay.Pay()方法会失败  
                        synchronized (lock){  
 <strong>                           if (mAlixPay == null)  
                                lock.wait (); </strong> 
                        }  

第一个问题：用Integer做lock对象。

在Oracle JDK里，Integer是有会缓存的，从-128 到127的Integer会缓存到内部的一个IntegerCache，所以两个Integer可能会是同一个对象。还可以用-XX:AutoBoxCacheMax参数来设置cache的范围。

另外，即使Integer的内部实现是没有缓存的，对于像Integer lock = 0; 这样的代码，编绎器可能会把它变成一个变量，然后共享这样一个常量。

所以可能不同的类，会共享了一个synchronized (lock)对象，线程的并发度大大降低，甚至可能会有死锁。

同理，像Boolean，Float，Long这种类型都是不适合用来做lock对象的。

最好的办法是直接 Object lock = new Object(); 。

第二个问题：wait方法没有在while循环里。

绝大部分情况下，object.wait()方法都应该while循环来判断条件变量。因为wait()函数可能会因为spurious wakeup而返回。

spurious wakeup请参考另一篇blog：http://blog.csdn.net/hengyunabc/article/details/27969613

另外直接看JDK的文档，里面就说得很清楚：

//As in the one argument version, interrupts and spurious wakeups are possible, and this method should always be used in a loop: 
     synchronized (obj) {
         while (<condition does not hold>)
             obj.wait();
         ... // Perform action appropriate to condition
     }

https://openhome.alipay.com/doc/docIndex.htm