diff options
author | Hung-ying Tyan <tyanh@google.com> | 2010-09-30 07:49:35 +0800 |
---|---|---|
committer | Hung-ying Tyan <tyanh@google.com> | 2010-09-30 08:10:17 +0800 |
commit | fb3a98b1d8d0ad040980d509c4c5341928b9460b (patch) | |
tree | bcacca587eefe97de61849caeb663e8ea053e92d /voip | |
parent | 320cdcb122505ba703326a102f9b13d2f2f8847a (diff) | |
download | frameworks_base-fb3a98b1d8d0ad040980d509c4c5341928b9460b.zip frameworks_base-fb3a98b1d8d0ad040980d509c4c5341928b9460b.tar.gz frameworks_base-fb3a98b1d8d0ad040980d509c4c5341928b9460b.tar.bz2 |
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
Diffstat (limited to 'voip')
-rw-r--r-- | voip/java/android/net/sip/SipManager.java | 4 | ||||
-rw-r--r-- | voip/java/com/android/server/sip/SipService.java | 131 | ||||
-rw-r--r-- | voip/java/com/android/server/sip/SipSessionGroup.java | 17 |
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; } |