From 33f349ef015b8c681aa448e4a497f3c9e316d4cb Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 20 Nov 2024 10:39:49 +0100 Subject: [PATCH] fix some race conditions in jingle ft --- .../jingle/JingleFileTransferConnection.java | 35 ++++++++++++------- .../WebRTCDataChannelTransport.java | 4 ++- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleFileTransferConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleFileTransferConnection.java index 712b7ccb2fa1bec643de6554d64155d65ef91f92..e2f8be3393a806a2eb4fc0dc58569db8c61cccd9 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleFileTransferConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleFileTransferConnection.java @@ -205,8 +205,7 @@ public class JingleFileTransferConnection extends AbstractJingleConnection if (transition( State.SESSION_INITIALIZED, () -> this.initiatorFileTransferContentMap = contentMap)) { - final var iq = - contentMap.toJinglePacket(Jingle.Action.SESSION_INITIATE, id.sessionId); + final var iq = contentMap.toJinglePacket(Jingle.Action.SESSION_INITIATE, id.sessionId); final var jingle = iq.getExtension(Jingle.class); if (xmppAxolotlMessage != null) { this.transportSecurity = @@ -456,8 +455,7 @@ public class JingleFileTransferConnection extends AbstractJingleConnection private void sendSessionAccept(final FileTransferContentMap contentMap) { setLocalContentMap(contentMap); - final var iq = - contentMap.toJinglePacket(Jingle.Action.SESSION_ACCEPT, id.sessionId); + final var iq = contentMap.toJinglePacket(Jingle.Action.SESSION_ACCEPT, id.sessionId); send(iq); // this needs to come after session-accept or else our candidate-error might arrive first this.transport.connect(); @@ -562,7 +560,7 @@ public class JingleFileTransferConnection extends AbstractJingleConnection Log.d(Config.LOGTAG, "peer confirmed received " + received); } - private void receiveSessionTerminate(final Iq jinglePacket, final Jingle jingle) { + private synchronized void receiveSessionTerminate(final Iq jinglePacket, final Jingle jingle) { respondOk(jinglePacket); final Jingle.ReasonWrapper wrapper = jingle.getReason(); final State previous = this.state; @@ -589,7 +587,6 @@ public class JingleFileTransferConnection extends AbstractJingleConnection } terminateTransport(); final State target = reasonToState(wrapper.reason); - // TODO check if we were already terminated transitionOrThrow(target); finish(); } @@ -865,13 +862,20 @@ public class JingleFileTransferConnection extends AbstractJingleConnection @Override public void onTransportEstablished() { - Log.d(Config.LOGTAG, "on transport established"); + Log.d(Config.LOGTAG, "transport established"); final AbstractFileTransceiver fileTransceiver; try { fileTransceiver = setupTransceiver(isResponder()); } catch (final Exception e) { - Log.d(Config.LOGTAG, "failed to set up file transceiver", e); - sendSessionTerminate(Reason.ofThrowable(e), e.getMessage()); + terminateTransport(); + if (isTerminated()) { + Log.d( + Config.LOGTAG, + "failed to set up file transceiver but session has already been terminated"); + } else { + Log.d(Config.LOGTAG, "failed to set up file transceiver", e); + sendSessionTerminate(Reason.ofThrowable(e), e.getMessage()); + } return; } this.fileTransceiver = fileTransceiver; @@ -951,6 +955,10 @@ public class JingleFileTransferConnection extends AbstractJingleConnection } private AbstractFileTransceiver setupTransceiver(final boolean receiving) throws IOException { + final var transport = this.transport; + if (transport == null) { + throw new IOException("No transport configured"); + } final var fileDescription = getLocalContentMap().requireOnlyFile(); final File file = xmppConnectionService.getFileBackend().getFile(message); final Runnable updateRunnable = () -> jingleConnectionManager.updateConversationUi(false); @@ -987,7 +995,8 @@ public class JingleFileTransferConnection extends AbstractJingleConnection private void sendSessionInfo(final FileTransferDescription.SessionInfo sessionInfo) { final var iq = new Iq(Iq.Type.SET); - final var jinglePacket = iq.addExtension(new Jingle(Jingle.Action.SESSION_INFO, this.id.sessionId)); + final var jinglePacket = + iq.addExtension(new Jingle(Jingle.Action.SESSION_INFO, this.id.sessionId)); jinglePacket.addChild(sessionInfo.asElement()); send(iq); } @@ -1071,8 +1080,7 @@ public class JingleFileTransferConnection extends AbstractJingleConnection + contentName); return; } - final Iq iq = - transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId); + final Iq iq = transportInfo.toJinglePacket(Jingle.Action.TRANSPORT_INFO, id.sessionId); send(iq); } @@ -1255,7 +1263,8 @@ public class JingleFileTransferConnection extends AbstractJingleConnection } terminateTransport(); final Iq iq = new Iq(Iq.Type.SET); - final var jingle = iq.addExtension(new Jingle(Jingle.Action.SESSION_TERMINATE, id.sessionId)); + final var jingle = + iq.addExtension(new Jingle(Jingle.Action.SESSION_TERMINATE, id.sessionId)); jingle.setReason(reason, "User requested to stop file transfer"); send(iq); finish(); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/transports/WebRTCDataChannelTransport.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/transports/WebRTCDataChannelTransport.java index 904c4aba89568f863bef293db89959cf58e187c7..95b15438460b50ac229ed522162e714f820a48d6 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/transports/WebRTCDataChannelTransport.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/transports/WebRTCDataChannelTransport.java @@ -22,6 +22,7 @@ import eu.siacs.conversations.xmpp.jingle.IceServers; import eu.siacs.conversations.xmpp.jingle.WebRTCWrapper; import eu.siacs.conversations.xmpp.jingle.stanzas.IceUdpTransportInfo; import eu.siacs.conversations.xmpp.jingle.stanzas.WebRTCDataChannelTransportInfo; + import im.conversations.android.xmpp.model.stanza.Iq; import org.webrtc.CandidatePairChangeEvent; @@ -45,6 +46,7 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Queue; +import java.util.concurrent.CancellationException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -455,7 +457,7 @@ public class WebRTCDataChannelTransport implements Transport { if (future != null && future.isDone()) { try { return future.get(); - } catch (final InterruptedException | ExecutionException e) { + } catch (final InterruptedException | ExecutionException | CancellationException e) { throw new WebRTCWrapper.PeerConnectionNotInitialized(); } } else {