rtp session propsoal: fix race condition with very fast 'busy' or 'error'

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/services/CallIntegrationConnectionService.java |  1 
src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java                     | 22 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java       | 52 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java           | 10 
4 files changed, 67 insertions(+), 18 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/services/CallIntegrationConnectionService.java 🔗

@@ -131,6 +131,7 @@ public class CallIntegrationConnectionService extends ConnectionService {
                 intent.putExtra(
                         RtpSessionActivity.EXTRA_LAST_REPORTED_STATE,
                         RtpEndUserState.FINDING_DEVICE.toString());
+                intent.putExtra(RtpSessionActivity.EXTRA_PROPOSED_SESSION_ID, proposal.sessionId);
                 callIntegration = proposal.getCallIntegration();
             }
             if (Media.audioOnly(media)) {

src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java 🔗

@@ -80,6 +80,7 @@ public class RtpSessionActivity extends XmppActivity
 
     public static final String EXTRA_WITH = "with";
     public static final String EXTRA_SESSION_ID = "session_id";
+    public static final String EXTRA_PROPOSED_SESSION_ID = "proposed_session_id";
     public static final String EXTRA_LAST_REPORTED_STATE = "last_reported_state";
     public static final String EXTRA_LAST_ACTION = "last_action";
     public static final String ACTION_ACCEPT_CALL = "action_accept_call";
@@ -386,7 +387,6 @@ public class RtpSessionActivity extends XmppActivity
         }
     }
 
-
     private void putScreenInCallMode() {
         putScreenInCallMode(requireRtpConnection().getMedia());
     }
@@ -509,6 +509,16 @@ public class RtpSessionActivity extends XmppActivity
             proposeJingleRtpSession(account, with, actionToMedia(action));
             setWith(account.getRoster().getContact(with), null);
         } else if (Intent.ACTION_VIEW.equals(action)) {
+            final String proposedSessionId = intent.getStringExtra(EXTRA_PROPOSED_SESSION_ID);
+            final JingleConnectionManager.TerminatedRtpSession terminatedRtpSession =
+                    xmppConnectionService
+                            .getJingleConnectionManager()
+                            .getTerminalSessionState(with, proposedSessionId);
+            if (terminatedRtpSession != null) {
+                // termination (due to message error or 'busy' was faster than opening the activity
+                initializeWithTerminatedSessionState(account, with, terminatedRtpSession);
+                return;
+            }
             final String extraLastState = intent.getStringExtra(EXTRA_LAST_REPORTED_STATE);
             final String lastAction = intent.getStringExtra(EXTRA_LAST_ACTION);
             final Set<Media> media = actionToMedia(lastAction);
@@ -1007,7 +1017,7 @@ public class RtpSessionActivity extends XmppActivity
     private void updateInCallButtonConfiguration(
             final RtpEndUserState state, final Set<Media> media) {
         if (STATES_CONSIDERED_CONNECTED.contains(state) && !isPictureInPicture()) {
-            Preconditions.checkArgument(media.size() > 0, "Media must not be empty");
+            Preconditions.checkArgument(!media.isEmpty(), "Media must not be empty");
             if (media.contains(Media.VIDEO)) {
                 final JingleRtpConnection rtpConnection = requireRtpConnection();
                 updateInCallButtonConfigurationVideo(
@@ -1028,7 +1038,13 @@ public class RtpSessionActivity extends XmppActivity
         } else if (STATES_SHOWING_SPEAKER_CONFIGURATION.contains(state)
                 && !isPictureInPicture()
                 && Media.audioOnly(media)) {
-            final CallIntegration callIntegration = requireCallIntegration();
+            final CallIntegration callIntegration;
+            try {
+                callIntegration = requireCallIntegration();
+            } catch (final IllegalStateException e) {
+                Log.e(Config.LOGTAG, "can not update InCallButtonConfiguration in state " + state);
+                return;
+            }
             updateInCallButtonConfigurationSpeaker(
                     callIntegration.getSelectedAudioDevice(),
                     callIntegration.getAudioDevices().size());

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java 🔗

@@ -505,6 +505,7 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                     getRtpSessionProposal(account, from.asBareJid(), sessionId);
             synchronized (rtpSessionProposals) {
                 if (proposal != null) {
+                    setTerminalSessionState(proposal, RtpEndUserState.DECLINED_OR_BUSY);
                     rtpSessionProposals.remove(proposal);
                     proposal.callIntegration.busy();
                     writeLogMissedOutgoing(
@@ -938,7 +939,12 @@ public class JingleConnectionManager extends AbstractConnectionManager {
             final DeviceDiscoveryState currentState =
                     sessionProposal == null ? null : rtpSessionProposals.get(sessionProposal);
             if (currentState == null) {
-                Log.d(Config.LOGTAG, "unable to find session proposal for session id " + sessionId);
+                Log.d(
+                        Config.LOGTAG,
+                        "unable to find session proposal for session id "
+                                + sessionId
+                                + " target="
+                                + target);
                 return;
             }
             if (currentState == DeviceDiscoveryState.DISCOVERED) {
@@ -947,14 +953,7 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                         "session proposal already at discovered. not going to fall back");
                 return;
             }
-            this.rtpSessionProposals.put(sessionProposal, target);
-            final RtpEndUserState endUserState = target.toEndUserState();
-            if (endUserState == RtpEndUserState.RINGING) {
-                sessionProposal.callIntegration.setDialing();
-            }
-            // toneManager.transition(endUserState, sessionProposal.media);
-            mXmppConnectionService.notifyJingleRtpConnectionUpdate(
-                    account, sessionProposal.with, sessionProposal.sessionId, endUserState);
+
             Log.d(
                     Config.LOGTAG,
                     account.getJid().asBareJid()
@@ -962,6 +961,30 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                             + sessionId
                             + " as "
                             + target);
+
+            final RtpEndUserState endUserState = target.toEndUserState();
+
+            if (target == DeviceDiscoveryState.FAILED) {
+                Log.d(Config.LOGTAG, "removing session proposal after failure");
+                setTerminalSessionState(sessionProposal, endUserState);
+                this.rtpSessionProposals.remove(sessionProposal);
+                sessionProposal.getCallIntegration().error();
+                mXmppConnectionService.notifyJingleRtpConnectionUpdate(
+                        account,
+                        sessionProposal.with,
+                        sessionProposal.sessionId,
+                        endUserState);
+                return;
+            }
+
+            this.rtpSessionProposals.put(sessionProposal, target);
+
+            if (endUserState == RtpEndUserState.RINGING) {
+                sessionProposal.callIntegration.setDialing();
+            }
+
+            mXmppConnectionService.notifyJingleRtpConnectionUpdate(
+                    account, sessionProposal.with, sessionProposal.sessionId, endUserState);
         }
     }
 
@@ -1020,6 +1043,11 @@ public class JingleConnectionManager extends AbstractConnectionManager {
                 PersistableSessionId.of(id), new TerminatedRtpSession(state, media));
     }
 
+    void setTerminalSessionState(final RtpSessionProposal proposal, final RtpEndUserState state) {
+        this.terminatedSessions.put(
+                PersistableSessionId.of(proposal), new TerminatedRtpSession(state, proposal.media));
+    }
+
     public TerminatedRtpSession getTerminalSessionState(final Jid with, final String sessionId) {
         return this.terminatedSessions.getIfPresent(new PersistableSessionId(with, sessionId));
     }
@@ -1033,10 +1061,14 @@ public class JingleConnectionManager extends AbstractConnectionManager {
             this.sessionId = sessionId;
         }
 
-        public static PersistableSessionId of(AbstractJingleConnection.Id id) {
+        public static PersistableSessionId of(final AbstractJingleConnection.Id id) {
             return new PersistableSessionId(id.with, id.sessionId);
         }
 
+        public static PersistableSessionId of(final RtpSessionProposal proposal) {
+            return new PersistableSessionId(proposal.with, proposal.sessionId);
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java 🔗

@@ -2708,14 +2708,14 @@ public class JingleRtpConnection extends AbstractJingleConnection
     }
 
     @Override
-    public void onCallIntegrationShowIncomingCallUi() {
+    public synchronized void onCallIntegrationShowIncomingCallUi() {
         if (isTerminated()) {
             // there might be race conditions with the call integration service invoking this
-            // callback when the rtp session has already ended. It should be enough to just return
-            // instead of throwing an exception. however throwing an exception gives us a sense of
-            // if and how frequently this happens
-            throw new IllegalStateException(
+            // callback when the rtp session has already ended.
+            Log.w(
+                    Config.LOGTAG,
                     "CallIntegration requested incoming call UI but session was already terminated");
+            return;
         }
         // TODO apparently this can be called too early as well?
         xmppConnectionService.getNotificationService().startRinging(id, getMedia());