From cb37321ecb39f04aa0c3b7f19010938304d11b3c Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 30 Mar 2024 11:15:41 +0100 Subject: [PATCH] rtp session propsoal: fix race condition with very fast 'busy' or 'error' --- .../CallIntegrationConnectionService.java | 1 + .../conversations/ui/RtpSessionActivity.java | 22 ++++++-- .../xmpp/jingle/JingleConnectionManager.java | 52 +++++++++++++++---- .../xmpp/jingle/JingleRtpConnection.java | 10 ++-- 4 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/services/CallIntegrationConnectionService.java b/src/main/java/eu/siacs/conversations/services/CallIntegrationConnectionService.java index b7cf00474..e86f6c4f8 100644 --- a/src/main/java/eu/siacs/conversations/services/CallIntegrationConnectionService.java +++ b/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)) { diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index 793ef16f9..871b7c259 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/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 = actionToMedia(lastAction); @@ -1007,7 +1017,7 @@ public class RtpSessionActivity extends XmppActivity private void updateInCallButtonConfiguration( final RtpEndUserState state, final Set 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()); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java index c94ed0ab1..ce3628697 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnectionManager.java +++ b/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; diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index ab9659185..bb7c70839 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/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());