SIP: misc fixes.

+ Fix keepalive timer event leak due to the race between stopping timer and
  the async'ed timeout handler
+ SipSessionImpl: set state before handling an event to ensure we get correct
  state when some error occurs during handling the event.
+ Fix potential NPE in SipManager.ListenerRelay.getUri().

Change-Id: I021ee34f83059fd4fbb64b30bea427a5462aa51b

3 files changed, 94 insertions, 58 deletions

diff --git a/voip/java/android/net/sip/SipManager.java b/voip/java/android/net/sip/SipManager.java
index 59631c1..52f5716 100644
--- a/voip/java/android/net/sip/SipManager.java
+++ b/voip/java/android/net/sip/SipManager.java
@@ -520,7 +520,9 @@ public class SipManager {
 
         private String getUri(ISipSession session) {
             try {
-                return session.getLocalProfile().getUriString();
+                return ((session == null)
+                        ? "no session"
+                        : session.getLocalProfile().getUriString());
             } catch (RemoteException e) {
                 throw new RuntimeException(e);
             }
diff --git a/voip/java/com/android/server/sip/SipService.java b/voip/java/com/android/server/sip/SipService.java
index 0ff5586..130fe9f 100644
--- a/voip/java/com/android/server/sip/SipService.java
+++ b/voip/java/com/android/server/sip/SipService.java
@@ -510,31 +510,43 @@ public final class SipService extends ISipService.Stub {
         }
     }
 
+    // KeepAliveProcess is controlled by AutoRegistrationProcess.
+    // All methods will be invoked in sync with SipService.this except realRun()
     private class KeepAliveProcess implements Runnable {
         private static final String TAG = "\\KEEPALIVE/";
         private static final int INTERVAL = 10;
         private SipSessionGroup.SipSessionImpl mSession;
+        private boolean mRunning = false;
 
         public KeepAliveProcess(SipSessionGroup.SipSessionImpl session) {
             mSession = session;
         }
 
         public void start() {
+            if (mRunning) return;
+            mRunning = true;
             mTimer.set(INTERVAL * 1000, this);
         }
 
+        // timeout handler
         public void run() {
+            if (!mRunning) return;
+            final SipSessionGroup.SipSessionImpl session = mSession;
+
             // delegate to mExecutor
             getExecutor().addTask(new Runnable() {
                 public void run() {
-                    realRun();
+                    realRun(session);
                 }
             });
         }
 
-        private void realRun() {
+        // real timeout handler
+        private void realRun(SipSessionGroup.SipSessionImpl session) {
             synchronized (SipService.this) {
-                SipSessionGroup.SipSessionImpl session = mSession.duplicate();
+                if (notCurrentSession(session)) return;
+
+                session = session.duplicate();
                 if (DEBUG) Log.d(TAG, "~~~ keepalive");
                 mTimer.cancel(this);
                 session.sendKeepAlive();
@@ -547,8 +559,14 @@ public final class SipService extends ISipService.Stub {
         }
 
         public void stop() {
+            mRunning = false;
+            mSession = null;
             mTimer.cancel(this);
         }
+
+        private boolean notCurrentSession(ISipSession session) {
+            return (session != mSession) || !mRunning;
+        }
     }
 
     private class AutoRegistrationProcess extends SipSessionAdapter
@@ -561,13 +579,15 @@ public final class SipService extends ISipService.Stub {
         private long mExpiryTime;
         private int mErrorCode;
         private String mErrorMessage;
+        private boolean mRunning = false;
 
         private String getAction() {
             return toString();
         }
 
         public void start(SipSessionGroup group) {
-            if (mSession == null) {
+            if (!mRunning) {
+                mRunning = true;
                 mBackoff = 1;
                 mSession = (SipSessionGroup.SipSessionImpl)
                         group.createSession(this);
@@ -584,35 +604,24 @@ public final class SipService extends ISipService.Stub {
         }
 
         public void stop() {
-            stop(false);
-        }
-
-        private void stopButKeepStates() {
-            stop(true);
-        }
-
-        private void stop(boolean keepStates) {
-            if (mSession == null) return;
+            if (!mRunning) return;
+            mRunning = false;
+            mSession.setListener(null);
             if (mConnected && mRegistered) mSession.unregister();
+
             mTimer.cancel(this);
             if (mKeepAliveProcess != null) {
                 mKeepAliveProcess.stop();
                 mKeepAliveProcess = null;
             }
-            if (!keepStates) {
-                mSession = null;
-                mRegistered = false;
-            }
-        }
 
-        private boolean isStopped() {
-            return (mSession == null);
+            mRegistered = false;
+            setListener(mProxy.getListener());
         }
 
         public void setListener(ISipSessionListener listener) {
             synchronized (SipService.this) {
                 mProxy.setListener(listener);
-                if (mSession == null) return;
 
                 try {
                     int state = (mSession == null)
@@ -632,6 +641,18 @@ public final class SipService extends ISipService.Stub {
                             mProxy.onRegistrationFailed(mSession, mErrorCode,
                                     mErrorMessage);
                         }
+                    } else if (!mConnected) {
+                        mProxy.onRegistrationFailed(mSession,
+                                SipErrorCode.DATA_CONNECTION_LOST,
+                                "no data connection");
+                    } else if (!mRunning) {
+                        mProxy.onRegistrationFailed(mSession,
+                                SipErrorCode.CLIENT_ERROR,
+                                "registration not running");
+                    } else {
+                        mProxy.onRegistrationFailed(mSession,
+                                SipErrorCode.IN_PROGRESS,
+                                String.valueOf(state));
                     }
                 } catch (Throwable t) {
                     Log.w(TAG, "setListener(): " + t);
@@ -643,21 +664,29 @@ public final class SipService extends ISipService.Stub {
             return mRegistered;
         }
 
+        // timeout handler
         public void run() {
-            // delegate to mExecutor
-            getExecutor().addTask(new Runnable() {
-                public void run() {
-                    realRun();
-                }
-            });
+            synchronized (SipService.this) {
+                if (!mRunning) return;
+                final SipSessionGroup.SipSessionImpl session = mSession;
+
+                // delegate to mExecutor
+                getExecutor().addTask(new Runnable() {
+                    public void run() {
+                        realRun(session);
+                    }
+                });
+            }
         }
 
-        private void realRun() {
-            mErrorCode = SipErrorCode.NO_ERROR;
-            mErrorMessage = null;
-            if (DEBUG) Log.d(TAG, "~~~ registering");
+        // real timeout handler
+        private void realRun(SipSessionGroup.SipSessionImpl session) {
             synchronized (SipService.this) {
-                if (mConnected && !isStopped()) mSession.register(EXPIRY_TIME);
+                if (notCurrentSession(session)) return;
+                mErrorCode = SipErrorCode.NO_ERROR;
+                mErrorMessage = null;
+                if (DEBUG) Log.d(TAG, "~~~ registering");
+                if (mConnected) session.register(EXPIRY_TIME);
             }
         }
 
@@ -697,22 +726,29 @@ public final class SipService extends ISipService.Stub {
         public void onRegistering(ISipSession session) {
             if (DEBUG) Log.d(TAG, "onRegistering(): " + session);
             synchronized (SipService.this) {
-                if (!isStopped() && (session != mSession)) return;
+                if (notCurrentSession(session)) return;
+
                 mRegistered = false;
                 mProxy.onRegistering(session);
             }
         }
 
+        private boolean notCurrentSession(ISipSession session) {
+            if (session != mSession) {
+                ((SipSessionGroup.SipSessionImpl) session).setListener(null);
+                return true;
+            }
+            return !mRunning;
+        }
+
         @Override
         public void onRegistrationDone(ISipSession session, int duration) {
             if (DEBUG) Log.d(TAG, "onRegistrationDone(): " + session);
             synchronized (SipService.this) {
-                if (!isStopped() && (session != mSession)) return;
+                if (notCurrentSession(session)) return;
 
                 mProxy.onRegistrationDone(session, duration);
 
-                if (isStopped()) return;
-
                 if (duration > 0) {
                     mSession.clearReRegisterRequired();
                     mExpiryTime = SystemClock.elapsedRealtime()
@@ -751,17 +787,18 @@ public final class SipService extends ISipService.Stub {
             if (DEBUG) Log.d(TAG, "onRegistrationFailed(): " + session + ": "
                     + SipErrorCode.toString(errorCode) + ": " + message);
             synchronized (SipService.this) {
-                if (!isStopped() && (session != mSession)) return;
-                mErrorCode = errorCode;
-                mErrorMessage = message;
-                mProxy.onRegistrationFailed(session, errorCode, message);
+                if (notCurrentSession(session)) return;
 
                 if (errorCode == SipErrorCode.INVALID_CREDENTIALS) {
                     if (DEBUG) Log.d(TAG, "   pause auto-registration");
-                    stopButKeepStates();
-                } else if (!isStopped()) {
+                    stop();
+                } else {
                     onError();
                 }
+
+                mErrorCode = errorCode;
+                mErrorMessage = message;
+                mProxy.onRegistrationFailed(session, errorCode, message);
             }
         }
 
@@ -769,14 +806,11 @@ public final class SipService extends ISipService.Stub {
         public void onRegistrationTimeout(ISipSession session) {
             if (DEBUG) Log.d(TAG, "onRegistrationTimeout(): " + session);
             synchronized (SipService.this) {
-                if (!isStopped() && (session != mSession)) return;
+                if (notCurrentSession(session)) return;
+
                 mErrorCode = SipErrorCode.TIME_OUT;
                 mProxy.onRegistrationTimeout(session);
-
-                if (!isStopped()) {
-                    mRegistered = false;
-                    onError();
-                }
+                onError();
             }
         }
 
@@ -883,6 +917,7 @@ public final class SipService extends ISipService.Stub {
                 mConnected = connected;
             }
 
+            // timeout handler
             @Override
             public void run() {
                 // delegate to mExecutor
diff --git a/voip/java/com/android/server/sip/SipSessionGroup.java b/voip/java/com/android/server/sip/SipSessionGroup.java
index 4321d7b..c68fa1b 100644
--- a/voip/java/com/android/server/sip/SipSessionGroup.java
+++ b/voip/java/com/android/server/sip/SipSessionGroup.java
@@ -334,12 +334,12 @@ class SipSessionGroup implements SipListener {
             if (isRequestEvent(Request.INVITE, evt)) {
                 RequestEvent event = (RequestEvent) evt;
                 SipSessionImpl newSession = new SipSessionImpl(mProxy);
+                newSession.mState = SipSession.State.INCOMING_CALL;
                 newSession.mServerTransaction = mSipHelper.sendRinging(event,
                         generateTag());
                 newSession.mDialog = newSession.mServerTransaction.getDialog();
                 newSession.mInviteReceived = event;
                 newSession.mPeerProfile = createPeerProfile(event.getRequest());
-                newSession.mState = SipSession.State.INCOMING_CALL;
                 newSession.mPeerSessionDescription =
                         extractContent(event.getRequest());
                 addSipSession(newSession);
@@ -708,7 +708,6 @@ class SipSessionGroup implements SipListener {
                 case SipSession.State.PINGING:
                     reset();
                     mReRegisterFlag = true;
-                    mState = SipSession.State.READY_TO_CALL;
                     break;
 
                 default:
@@ -877,6 +876,7 @@ class SipSessionGroup implements SipListener {
         private boolean readyForCall(EventObject evt) throws SipException {
             // expect MakeCallCommand, RegisterCommand, DEREGISTER
             if (evt instanceof MakeCallCommand) {
+                mState = SipSession.State.OUTGOING_CALL;
                 MakeCallCommand cmd = (MakeCallCommand) evt;
                 mPeerProfile = cmd.getPeerProfile();
                 mClientTransaction = mSipHelper.sendInvite(mLocalProfile,
@@ -884,25 +884,24 @@ class SipSessionGroup implements SipListener {
                         generateTag());
                 mDialog = mClientTransaction.getDialog();
                 addSipSession(this);
-                mState = SipSession.State.OUTGOING_CALL;
                 mProxy.onCalling(this);
                 startSessionTimer(cmd.getTimeout());
                 return true;
             } else if (evt instanceof RegisterCommand) {
+                mState = SipSession.State.REGISTERING;
                 int duration = ((RegisterCommand) evt).getDuration();
                 mClientTransaction = mSipHelper.sendRegister(mLocalProfile,
                         generateTag(), duration);
                 mDialog = mClientTransaction.getDialog();
                 addSipSession(this);
-                mState = SipSession.State.REGISTERING;
                 mProxy.onRegistering(this);
                 return true;
             } else if (DEREGISTER == evt) {
+                mState = SipSession.State.DEREGISTERING;
                 mClientTransaction = mSipHelper.sendRegister(mLocalProfile,
                         generateTag(), 0);
                 mDialog = mClientTransaction.getDialog();
                 addSipSession(this);
-                mState = SipSession.State.DEREGISTERING;
                 mProxy.onRegistering(this);
                 return true;
             }
@@ -913,11 +912,11 @@ class SipSessionGroup implements SipListener {
             // expect MakeCallCommand(answering) , END_CALL cmd , Cancel
             if (evt instanceof MakeCallCommand) {
                 // answer call
+                mState = SipSession.State.INCOMING_CALL_ANSWERING;
                 mServerTransaction = mSipHelper.sendInviteOk(mInviteReceived,
                         mLocalProfile,
                         ((MakeCallCommand) evt).getSessionDescription(),
                         mServerTransaction);
-                mState = SipSession.State.INCOMING_CALL_ANSWERING;
                 startSessionTimer(((MakeCallCommand) evt).getTimeout());
                 return true;
             } else if (END_CALL == evt) {
@@ -1009,8 +1008,8 @@ class SipSessionGroup implements SipListener {
                 // RFC says that UA should not send out cancel when no
                 // response comes back yet. We are cheating for not checking
                 // response.
-                mSipHelper.sendCancel(mClientTransaction);
                 mState = SipSession.State.OUTGOING_CALL_CANCELING;
+                mSipHelper.sendCancel(mClientTransaction);
                 startSessionTimer(CANCEL_CALL_TIMER);
                 return true;
             }
@@ -1065,8 +1064,8 @@ class SipSessionGroup implements SipListener {
                 return true;
             } else if (isRequestEvent(Request.INVITE, evt)) {
                 // got Re-INVITE
-                RequestEvent event = mInviteReceived = (RequestEvent) evt;
                 mState = SipSession.State.INCOMING_CALL;
+                RequestEvent event = mInviteReceived = (RequestEvent) evt;
                 mPeerSessionDescription = extractContent(event.getRequest());
                 mServerTransaction = null;
                 mProxy.onRinging(this, mPeerProfile, mPeerSessionDescription);
@@ -1077,9 +1076,9 @@ class SipSessionGroup implements SipListener {
                 return true;
             } else if (evt instanceof MakeCallCommand) {
                 // to change call
+                mState = SipSession.State.OUTGOING_CALL;
                 mClientTransaction = mSipHelper.sendReinvite(mDialog,
                         ((MakeCallCommand) evt).getSessionDescription());
-                mState = SipSession.State.OUTGOING_CALL;
                 startSessionTimer(((MakeCallCommand) evt).getTimeout());
                 return true;
             }