Fix for BT turn on/off issues: NPE, FD/thread leaks, ANR.

This fix addresses numerous BT on/off issues found from the CTS test.
Fixed fD/pipe leak in various state machine Looper objects by calling quit() function.
Fixed cursor leak in AtPhonebook.
Change Pbap to stop the RFCOMM listener on the STATE_TURNING_OFF instead of STATE_OFF.
Removed several cleanup() methods causing NPE.
Reduced RFCOMM listener timeout from 3 seconds to 300ms between retry.
bug 6834336

Change-Id: I52423343d8f52f65294af0143f373871a9874d77

diff --git a/src/com/android/bluetooth/a2dp/A2dpService.java b/src/com/android/bluetooth/a2dp/A2dpService.java
index 4835890..f223db9 100755 (executable)
--- a/src/com/android/bluetooth/a2dp/A2dpService.java
+++ b/src/com/android/bluetooth/a2dp/A2dpService.java
@@ -41,14 +41,13 @@ public class A2dpService extends ProfileService {
     }
 
     protected boolean stop() {
-        // TODO(BT) mStateMachine.quit();
+        mStateMachine.doQuit();
         return true;
     }
 
     protected boolean cleanup() {
         if (mStateMachine!= null) {
             mStateMachine.cleanup();
-            mStateMachine=null;
         }
         return true;
     }

diff --git a/src/com/android/bluetooth/a2dp/A2dpStateMachine.java b/src/com/android/bluetooth/a2dp/A2dpStateMachine.java
index 8d42512..9b67eed 100644 (file)
--- a/src/com/android/bluetooth/a2dp/A2dpStateMachine.java
+++ b/src/com/android/bluetooth/a2dp/A2dpStateMachine.java
@@ -130,14 +130,12 @@ final class A2dpStateMachine extends StateMachine {
 
     }
 
+    public void doQuit() {
+        quitNow();
+    }
+
     public void cleanup() {
         cleanupNative();
-        if(mService != null)
-            mService = null;
-        if (mContext != null)
-            mContext = null;
-        if(mAdapter != null)
-            mAdapter = null;
     }
 
         private class Disconnected extends State {

diff --git a/src/com/android/bluetooth/btservice/AdapterService.java b/src/com/android/bluetooth/btservice/AdapterService.java
index 1e56129..2e1ede8 100755 (executable)
--- a/src/com/android/bluetooth/btservice/AdapterService.java
+++ b/src/com/android/bluetooth/btservice/AdapterService.java
@@ -325,20 +325,17 @@ public class AdapterService extends Service {
         mCleaningUp = true;
 
         if (mAdapterStateMachine != null) {
-            // TODO(BT) mAdapterStateMachine.quit();
+            mAdapterStateMachine.doQuit();
             mAdapterStateMachine.cleanup();
-            mAdapterStateMachine = null;
         }
 
         if (mBondStateMachine != null) {
-            // TODO(BT) mBondStateMachine.quit();
+            mBondStateMachine.doQuit();
             mBondStateMachine.cleanup();
-            mBondStateMachine = null;
         }
 
         if (mRemoteDevices != null) {
             mRemoteDevices.cleanup();
-            mRemoteDevices = null;
         }
 
         if (mNativeAvailable) {
@@ -350,24 +347,21 @@ public class AdapterService extends Service {
 
         if (mAdapterProperties != null) {
             mAdapterProperties.cleanup();
-            mAdapterProperties = null;
         }
 
         if (mJniCallbacks != null) {
             mJniCallbacks.cleanup();
-            mJniCallbacks = null;
         }
 
         if (mProfileServicesState != null) {
             mProfileServicesState.clear();
-            mProfileServicesState= null;
         }
 
         clearAdapterService();
 
         if (mBinder != null) {
             mBinder.cleanup();
-            mBinder = null;
+            mBinder = null;  //Do not remove. Otherwise Binder leak!
         }
 
         if (mCallbacks !=null) {

diff --git a/src/com/android/bluetooth/btservice/AdapterState.java b/src/com/android/bluetooth/btservice/AdapterState.java
index f6b4943..803a4c8 100755 (executable)
--- a/src/com/android/bluetooth/btservice/AdapterState.java
+++ b/src/com/android/bluetooth/btservice/AdapterState.java
@@ -80,6 +80,11 @@ final class AdapterState extends StateMachine {
         setInitialState(mOffState);
     }
 
+
+    public void doQuit() {
+        quitNow();
+    }
+
     public void cleanup() {
         if(mAdapterProperties != null)
             mAdapterProperties = null;
@@ -95,10 +100,6 @@ final class AdapterState extends StateMachine {
 
         @Override
         public boolean processMessage(Message msg) {
-            /* TODO(BT) if (msg.what == SM_QUIT_CMD) {
-                Log.d(TAG, "Received quit request...");
-                return false;
-                } */
 
             switch(msg.what) {
                case USER_TURN_ON:
@@ -129,6 +130,7 @@ final class AdapterState extends StateMachine {
 
         @Override
         public boolean processMessage(Message msg) {
+
             switch(msg.what) {
                case USER_TURN_OFF:
                    if (DBG) Log.d(TAG,"CURRENT_STATE=ON, MESSAGE = USER_TURN_OFF");
@@ -181,8 +183,10 @@ final class AdapterState extends StateMachine {
 
         @Override
         public boolean processMessage(Message msg) {
+
             boolean isTurningOn= isTurningOn();
             boolean isTurningOff = isTurningOff();
+
             switch (msg.what) {
                 case USER_TURN_ON:
                     if (DBG) Log.d(TAG,"CURRENT_STATE=PENDING, MESSAGE = USER_TURN_ON"
@@ -323,4 +327,5 @@ final class AdapterState extends StateMachine {
     private void errorLog(String msg) {
         Log.e(TAG, msg);
     }
+
 }

diff --git a/src/com/android/bluetooth/btservice/BondStateMachine.java b/src/com/android/bluetooth/btservice/BondStateMachine.java
index 4627b77..d69bd07 100755 (executable)
--- a/src/com/android/bluetooth/btservice/BondStateMachine.java
+++ b/src/com/android/bluetooth/btservice/BondStateMachine.java
@@ -59,6 +59,10 @@ final class BondStateMachine extends StateMachine {
         setInitialState(mStableState);
     }
 
+    public void doQuit() {
+        quitNow();
+    }
+
     public void cleanup() {
         mAdapterService = null;
         mRemoteDevices = null;
@@ -73,12 +77,6 @@ final class BondStateMachine extends StateMachine {
 
         @Override
         public boolean processMessage(Message msg) {
-            // TODO(BT)
-            // if (msg.what == SM_QUIT_CMD) {
-            //     Log.d(TAG, "Received quit request...");
-            //     return false;
-            // }
-
 
             BluetoothDevice dev = (BluetoothDevice)msg.obj;
 
@@ -126,11 +124,6 @@ final class BondStateMachine extends StateMachine {
 
         @Override
         public boolean processMessage(Message msg) {
-            // TODO(BT)
-            // if (msg.what == SM_QUIT_CMD) {
-            //     Log.d(TAG, "PendingCommandState(): Received quit request...");
-            //     return false;
-            // }
 
             BluetoothDevice dev = (BluetoothDevice)msg.obj;
             boolean result = false;
@@ -271,4 +264,5 @@ final class BondStateMachine extends StateMachine {
     private void errorLog(String msg) {
         Log.e(TAG, msg);
     }
+
 }

diff --git a/src/com/android/bluetooth/btservice/RemoteDevices.java b/src/com/android/bluetooth/btservice/RemoteDevices.java
index e9f4792..08ba36b 100755 (executable)
--- a/src/com/android/bluetooth/btservice/RemoteDevices.java
+++ b/src/com/android/bluetooth/btservice/RemoteDevices.java
@@ -47,12 +47,11 @@ final class RemoteDevices {
 
 
     void cleanup() {
-        mSdpTracker.clear();
-        mSdpTracker = null;
-        mDevices.clear();
-        mDevices = null;
-        mAdapterService = null;
-        mAdapter= null;
+        if (mSdpTracker !=null)
+            mSdpTracker.clear();
+
+        if (mDevices != null)
+            mDevices.clear();
     }
 
     public Object Clone() throws CloneNotSupportedException {

diff --git a/src/com/android/bluetooth/hdp/HealthService.java b/src/com/android/bluetooth/hdp/HealthService.java
index b843bc6..f689cfc 100755 (executable)
--- a/src/com/android/bluetooth/hdp/HealthService.java
+++ b/src/com/android/bluetooth/hdp/HealthService.java
@@ -114,15 +114,12 @@ public class HealthService extends ProfileService {
         }
         if(mHealthChannels != null) {
             mHealthChannels.clear();
-            mHealthChannels = null;
         }
         if(mHealthDevices != null) {
             mHealthDevices.clear();
-            mHealthDevices = null;
         }
         if(mApps != null) {
             mApps.clear();
-            mApps = null;
         }
         return true;
     }

diff --git a/src/com/android/bluetooth/hfp/AtPhonebook.java b/src/com/android/bluetooth/hfp/AtPhonebook.java
index dea2010..3f499ba 100755 (executable)
--- a/src/com/android/bluetooth/hfp/AtPhonebook.java
+++ b/src/com/android/bluetooth/hfp/AtPhonebook.java
@@ -113,9 +113,6 @@ public class AtPhonebook {
 
     public void cleanup() {
         mPhonebooks.clear();
-        mContentResolver = null;
-        mContext = null;
-        mStateMachine = null;
     }
 
     /** Returns the last dialled number, or null if no numbers have been called */
@@ -220,6 +217,8 @@ public class AtPhonebook {
                 }
                 int size = pbr.cursor.getCount();
                 atCommandResponse = "+CPBS: \"" + mCurrentPhonebook + "\"," + size + "," + getMaxPhoneBookSize(size);
+                pbr.cursor.close();
+                pbr.cursor = null;
                 atCommandResult = HeadsetHalConstants.AT_RESPONSE_OK;
                 break;
             case TYPE_TEST: // Test
@@ -282,6 +281,8 @@ public class AtPhonebook {
                     }
                     size = pbr.cursor.getCount();
                     log("handleCpbrCommand - size = "+size);
+                    pbr.cursor.close();
+                    pbr.cursor = null;
                 }
                 if (size == 0) {
                     /* Sending "+CPBR: (1-0)" can confused some carkits, send "1-1" * instead */
@@ -547,6 +548,10 @@ public class AtPhonebook {
                 break;
             }
         }
+        if(pbr != null && pbr.cursor != null) {
+            pbr.cursor.close();
+            pbr.cursor = null;
+        }
         return atCommandResult;
     }
 

diff --git a/src/com/android/bluetooth/hfp/HeadsetService.java b/src/com/android/bluetooth/hfp/HeadsetService.java
index 49fc3f5..9b0935d 100755 (executable)
--- a/src/com/android/bluetooth/hfp/HeadsetService.java
+++ b/src/com/android/bluetooth/hfp/HeadsetService.java
@@ -63,14 +63,13 @@ public class HeadsetService extends ProfileService {
         } catch (Exception e) {
             Log.w(TAG,"Unable to unregister headset receiver",e);
         }
-        // TODO(BT) mStateMachine.quit();
+        mStateMachine.doQuit();
         return true;
     }
 
     protected boolean cleanup() {
         if (mStateMachine != null) {
             mStateMachine.cleanup();
-            mStateMachine=null;
         }
         return true;
     }

diff --git a/src/com/android/bluetooth/hfp/HeadsetStateMachine.java b/src/com/android/bluetooth/hfp/HeadsetStateMachine.java
index a58349e..09684bb 100755 (executable)
--- a/src/com/android/bluetooth/hfp/HeadsetStateMachine.java
+++ b/src/com/android/bluetooth/hfp/HeadsetStateMachine.java
@@ -192,6 +192,11 @@ final class HeadsetStateMachine extends StateMachine {
         setInitialState(mDisconnected);
     }
 
+
+    public void doQuit() {
+        quitNow();
+    }
+
     public void cleanup() {
         if (mPhoneProxy != null) {
             if (DBG) Log.d(TAG,"Unbinding service...");
@@ -207,18 +212,14 @@ final class HeadsetStateMachine extends StateMachine {
         if (mPhoneState != null) {
             mPhoneState.listenForPhoneState(false);
             mPhoneState.cleanup();
-            mPhoneState=null;
         }
         if (mPhonebook != null) {
             mPhonebook.cleanup();
-            mPhonebook = null;
         }
         if (mNativeAvailable) {
             cleanupNative();
             mNativeAvailable = false;
         }
-        mService = null;
-        mAdapter = null;
     }
 
     private class Disconnected extends State {

diff --git a/src/com/android/bluetooth/hid/HidService.java b/src/com/android/bluetooth/hid/HidService.java
index 2ed02b7..5197f27 100755 (executable)
--- a/src/com/android/bluetooth/hid/HidService.java
+++ b/src/com/android/bluetooth/hid/HidService.java
@@ -84,7 +84,6 @@ public class HidService extends ProfileService {
 
         if(mInputDevices != null) {
             mInputDevices.clear();
-            mInputDevices = null;
         }
         return true;
     }

diff --git a/src/com/android/bluetooth/opp/BluetoothOppRfcommListener.java b/src/com/android/bluetooth/opp/BluetoothOppRfcommListener.java
index b5506f0..6109b5a 100755 (executable)
--- a/src/com/android/bluetooth/opp/BluetoothOppRfcommListener.java
+++ b/src/com/android/bluetooth/opp/BluetoothOppRfcommListener.java
@@ -53,10 +53,8 @@ public class BluetoothOppRfcommListener {
     private static final boolean V = Constants.VERBOSE;
 
     public static final int MSG_INCOMING_BTOPP_CONNECTION = 100;
-    private static final int JOIN_TIMEOUT_MS=2000;
 
     private volatile boolean mInterrupted;
-    private volatile boolean mFinish;
 
     private Thread mSocketAcceptThread;
 
@@ -123,11 +121,12 @@ public class BluetoothOppRfcommListener {
                                 Log.e(TAG, "Error create RfcommServerSocket " + e1);
                                 serverOK = false;
                             }
+
                             if (!serverOK) {
                                 synchronized (this) {
                                     try {
-                                        if (V) Log.v(TAG, "Wait 3 seconds");
-                                        Thread.sleep(3000);
+                                        if (V) Log.v(TAG, "Wait 300 ms");
+                                        Thread.sleep(300);
                                     } catch (InterruptedException e) {
                                         Log.e(TAG, "socketAcceptThread thread was interrupted (3)");
                                         mInterrupted = true;
@@ -169,6 +168,9 @@ public class BluetoothOppRfcommListener {
                                 }
                             } catch (IOException e) {
                                 Log.e(TAG, "Error accept connection " + e);
+                                try {
+                                    Thread.sleep(500);
+                                } catch (InterruptedException ie) {}
                             }
                         }
                         Log.i(TAG, "BluetoothSocket listen thread finished");

diff --git a/src/com/android/bluetooth/opp/BluetoothOppService.java b/src/com/android/bluetooth/opp/BluetoothOppService.java
index 750487b..5a30433 100755 (executable)
--- a/src/com/android/bluetooth/opp/BluetoothOppService.java
+++ b/src/com/android/bluetooth/opp/BluetoothOppService.java
@@ -333,43 +333,16 @@ public class BluetoothOppService extends Service {
         getContentResolver().unregisterContentObserver(mObserver);
         unregisterReceiver(mBluetoothReceiver);
         mSocketListener.stop();
-        _cleanup();
-    }
 
-    /* Cleanup all local references. Called during onDestroy */
-    private void _cleanup() {
         if(mBatchs != null) {
             mBatchs.clear();
-            mBatchs = null;
         }
         if(mShares != null) {
             mShares.clear();
-            mShares = null;
         }
-        if(mObserver != null)
-            mObserver = null;
-        if(mNotifier != null)
-            mNotifier = null;
         if(mHandler != null) {
             mHandler.removeCallbacksAndMessages(null);
-            mHandler = null;
-        }
-        if(mSocketListener != null)
-            mSocketListener = null;
-        if(mPowerManager != null)
-            mPowerManager = null;
-        if(mPendingConnection != null)
-            mPendingConnection = null;
-        if(mTransfer != null) {
-            mTransfer.cleanup();
-            mTransfer = null;
-        }
-        if(mServerTransfer!= null) {
-            mServerTransfer.cleanup();
-            mServerTransfer = null;
         }
-        if(mAdapter != null)
-            mAdapter = null;
     }
 
     /* suppose we auto accept an incoming OPUSH connection */
@@ -771,7 +744,6 @@ public class BluetoothOppService extends Service {
                         Log.e(TAG, "Unexpected error! batch id " + batch.mId
                                 + " doesn't match mTransfer id " + mTransfer.getBatchId());
                     }
-                    mTransfer.cleanup();
                     mTransfer = null;
                 } else {
                     if (mServerTransfer == null) {
@@ -783,7 +755,6 @@ public class BluetoothOppService extends Service {
                                 + " doesn't match mServerTransfer id "
                                 + mServerTransfer.getBatchId());
                     }
-                    mServerTransfer.cleanup();
                     mServerTransfer = null;
                 }
                 removeBatch(batch);

diff --git a/src/com/android/bluetooth/opp/BluetoothOppTransfer.java b/src/com/android/bluetooth/opp/BluetoothOppTransfer.java
index e345f4e..44d9bb6 100755 (executable)
--- a/src/com/android/bluetooth/opp/BluetoothOppTransfer.java
+++ b/src/com/android/bluetooth/opp/BluetoothOppTransfer.java
@@ -419,28 +419,6 @@ public class BluetoothOppTransfer implements BluetoothOppBatch.BluetoothOppBatch
         }
     }
 
-    /**
-     * Object cleanup
-     */
-    public void cleanup() {
-        if(mContext != null)
-            mContext = null;
-        if(mAdapter != null)
-            mAdapter = null;
-        if(mBatch != null) {
-            mBatch = null;
-        }
-        if(mSession != null)
-            mSession = null;
-        if(mCurrentShare != null)
-            mCurrentShare = null;
-        if(mTransport != null)
-            mTransport = null;
-        if(mSessionHandler != null) {
-            mSessionHandler = null;
-        }
-    }
-
     private void startObexSession() {
 
         mBatch.mStatus = Constants.BATCH_STATUS_RUNNING;

diff --git a/src/com/android/bluetooth/pan/PanService.java b/src/com/android/bluetooth/pan/PanService.java
index e851433..f5d5ff2 100755 (executable)
--- a/src/com/android/bluetooth/pan/PanService.java
+++ b/src/com/android/bluetooth/pan/PanService.java
@@ -105,14 +105,10 @@ public class PanService extends ProfileService {
                                                    BluetoothPan.LOCAL_PANU_ROLE, BluetoothPan.REMOTE_NAP_ROLE);
             }
             mPanDevices.clear();
-            mPanDevices = null;
         }
         if(mBluetoothIfaceAddresses != null) {
             mBluetoothIfaceAddresses.clear();
-            mBluetoothIfaceAddresses = null;
         }
-        if(mPanIfName != null)
-            mPanIfName = null;
         return true;
     }
 

diff --git a/src/com/android/bluetooth/pbap/BluetoothPbapReceiver.java b/src/com/android/bluetooth/pbap/BluetoothPbapReceiver.java
index 6aa09dc..2356e81 100644 (file)
--- a/src/com/android/bluetooth/pbap/BluetoothPbapReceiver.java
+++ b/src/com/android/bluetooth/pbap/BluetoothPbapReceiver.java
@@ -61,7 +61,9 @@ public class BluetoothPbapReceiver extends BroadcastReceiver {
             in.putExtra(BluetoothAdapter.EXTRA_STATE, state);
             if (V) Log.v(TAG,"***********state = " + state);
             if ((state == BluetoothAdapter.STATE_TURNING_ON)
-                    || (state == BluetoothAdapter.STATE_TURNING_OFF)) {
+                    || (state == BluetoothAdapter.STATE_OFF)) {
+                //FIX: We turn on PBAP after BluetoothAdapter.STATE_ON,
+                //but we turn off PBAP right after BluetoothAdapter.STATE_TURNING_OFF
                 startService = false;
             }
         } else {

diff --git a/src/com/android/bluetooth/pbap/BluetoothPbapService.java b/src/com/android/bluetooth/pbap/BluetoothPbapService.java
index 4c918e0..80cd8ee 100755 (executable)
--- a/src/com/android/bluetooth/pbap/BluetoothPbapService.java
+++ b/src/com/android/bluetooth/pbap/BluetoothPbapService.java
@@ -236,7 +236,7 @@ public class BluetoothPbapService extends Service {
 
         boolean removeTimeoutMsg = true;
         if (action.equals(BluetoothAdapter.ACTION_STATE_CHANGED)) {
-            if (state == BluetoothAdapter.STATE_OFF) {
+            if (state == BluetoothAdapter.STATE_TURNING_OFF) {
                 // Send any pending timeout now, as this service will be destroyed.
                 if (mSessionStatusHandler.hasMessages(USER_TIMEOUT)) {
                     Intent timeoutIntent =
@@ -340,8 +340,7 @@ public class BluetoothPbapService extends Service {
             try {
                 // It is mandatory for PSE to support initiation of bonding and
                 // encryption.
-                mServerSocket = mAdapter.listenUsingEncryptedRfcommWithServiceRecord(
-                                "OBEX Phonebook Access Server", BluetoothUuid.PBAP_PSE.getUuid());
+                mServerSocket = mAdapter.listenUsingEncryptedRfcommWithServiceRecord("OBEX Phonebook Access Server", BluetoothUuid.PBAP_PSE.getUuid());
 
             } catch (IOException e) {
                 Log.e(TAG, "Error create RfcommServerSocket " + e.toString());
@@ -350,8 +349,8 @@ public class BluetoothPbapService extends Service {
             if (!initSocketOK) {
                 synchronized (this) {
                     try {
-                        if (VERBOSE) Log.v(TAG, "wait 3 seconds");
-                        Thread.sleep(3000);
+                        if (VERBOSE) Log.v(TAG, "wait 300 ms");
+                        Thread.sleep(300);
                     } catch (InterruptedException e) {
                         Log.e(TAG, "socketAcceptThread thread was interrupted (3)");
                         mInterrupted = true;