From 7dfd47a5c423a9ea56b18b3829d5afe81c24ced2 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Sat, 18 Apr 2020 18:22:10 +0200 Subject: [PATCH] better crash than leave WebRTCWrapper unclosed --- .../xmpp/jingle/JingleRtpConnection.java | 41 +++++++++++-------- .../xmpp/jingle/WebRTCWrapper.java | 11 +++++ 2 files changed, 34 insertions(+), 18 deletions(-) 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 3067f18f0..d6e950692 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -193,7 +193,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web sendSessionTerminate(Reason.CONNECTIVITY_ERROR); } else { transitionOrThrow(State.TERMINATED_CONNECTIVITY_ERROR); - jingleConnectionManager.finishConnection(this); + finish(); } } @@ -213,7 +213,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (previous == State.PROPOSED || previous == State.SESSION_INITIALIZED) { xmppConnectionService.getNotificationService().cancelIncomingCallNotification(); } - jingleConnectionManager.finishConnection(this); + finish(); } private void receiveTransportInfo(final JinglePacket jinglePacket) { @@ -466,7 +466,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (transition(State.TERMINATED_CONNECTIVITY_ERROR)) { webRTCWrapper.close(); Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": transitioned into connectivity error"); - this.jingleConnectionManager.finishConnection(this); + this.finish(); } } @@ -481,7 +481,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web this.message.setCarbon(true); //indicate that call was accepted on other device this.writeLogMessageSuccess(0); this.xmppConnectionService.getNotificationService().cancelIncomingCallNotification(); - this.jingleConnectionManager.finishConnection(this); + this.finish(); } else { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": unable to transition to accept because already in state=" + this.state); } @@ -496,7 +496,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (originatedFromMyself) { if (transition(State.REJECTED)) { this.xmppConnectionService.getNotificationService().cancelIncomingCallNotification(); - this.jingleConnectionManager.finishConnection(this); + this.finish(); if (serverMsgId != null) { this.message.setServerMsgId(serverMsgId); } @@ -561,7 +561,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (transition(State.ACCEPTED)) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": moved session with " + id.with + " into state accepted after received carbon copied procced"); this.xmppConnectionService.getNotificationService().cancelIncomingCallNotification(); - this.jingleConnectionManager.finishConnection(this); + this.finish(); } } else { Log.d(Config.LOGTAG, String.format("%s: ignoring proceed from %s. was expected from %s", id.account.getJid().asBareJid(), from, id.with)); @@ -578,7 +578,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } this.message.setTime(timestamp); writeLogMessageMissed(); - jingleConnectionManager.finishConnection(this); + finish(); } else { Log.d(Config.LOGTAG, "ignoring retract because already in " + this.state); } @@ -641,7 +641,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web jinglePacket.setReason(reason, text); Log.d(Config.LOGTAG, jinglePacket.toString()); send(jinglePacket); - jingleConnectionManager.finishConnection(this); + finish(); } private void sendTransportInfo(final String contentName, IceUdpTransportInfo.Candidate candidate) { @@ -695,16 +695,16 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } this.webRTCWrapper.close(); transition(State.TERMINATED_CONNECTIVITY_ERROR); - this.jingleConnectionManager.finishConnection(this); + this.finish(); } } private void terminateWithOutOfOrder(final JinglePacket jinglePacket) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": terminating session with out-of-order"); - webRTCWrapper.close(); + this.webRTCWrapper.close(); transitionOrThrow(State.TERMINATED_APPLICATION_FAILURE); respondWithOutOfOrder(jinglePacket); - jingleConnectionManager.finishConnection(this); + this.finish(); } private void respondWithOutOfOrder(final JinglePacket jinglePacket) { @@ -820,13 +820,13 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } if (isInState(State.PROCEED)) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": ending call while in state PROCEED just means ending the connection"); - webRTCWrapper.close(); - jingleConnectionManager.finishConnection(this); + this.webRTCWrapper.close(); + this.finish(); transitionOrThrow(State.TERMINATED_SUCCESS); //arguably this wasn't success; but not a real failure either return; } if (isInitiator() && isInState(State.SESSION_INITIALIZED, State.SESSION_INITIALIZED_PRE_APPROVED)) { - webRTCWrapper.close(); + this.webRTCWrapper.close(); sendSessionTerminate(Reason.CANCEL); return; } @@ -835,7 +835,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return; } if (isInState(State.SESSION_INITIALIZED_PRE_APPROVED, State.SESSION_ACCEPTED)) { - webRTCWrapper.close(); + this.webRTCWrapper.close(); sendSessionTerminate(Reason.SUCCESS); return; } @@ -869,7 +869,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web writeLogMessageMissed(); xmppConnectionService.getNotificationService().cancelIncomingCallNotification(); this.sendJingleMessage("reject"); - jingleConnectionManager.finishConnection(this); + finish(); } private void rejectCallFromSessionInitiate() { @@ -1020,8 +1020,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web continue; } if (Arrays.asList("stun", "stuns", "turn", "turns").contains(type) && Arrays.asList("udp", "tcp").contains(transport)) { - if (Arrays.asList("stuns","turns").contains(type) && "udp".equals(transport)) { - Log.d(Config.LOGTAG,id.account.getJid().asBareJid()+": skipping invalid combination of udp/tls in external services"); + if (Arrays.asList("stuns", "turns").contains(type) && "udp".equals(transport)) { + Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": skipping invalid combination of udp/tls in external services"); continue; } //TODO wrap ipv6 addresses @@ -1054,6 +1054,11 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } + private void finish() { + this.webRTCWrapper.verifyClosed(); + this.jingleConnectionManager.finishConnection(this); + } + private void writeLogMessage(final State state) { final long started = this.rtpConnectionStarted; long duration = started <= 0 ? 0 : SystemClock.elapsedRealtime() - started; diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java index 28d0d8fa4..b6ed9aa10 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/WebRTCWrapper.java @@ -218,6 +218,7 @@ public class WebRTCWrapper { final EglBase eglBase = this.eglBase; if (peerConnection != null) { peerConnection.dispose(); + this.peerConnection = null; } if (audioManager != null) { mainHandler.post(audioManager::stop); @@ -233,6 +234,16 @@ public class WebRTCWrapper { } if (eglBase != null) { eglBase.release(); + this.eglBase = null; + } + } + + void verifyClosed() { + if (this.peerConnection != null + || this.eglBase != null + || this.localVideoTrack != null + || this.remoteVideoTrack != null) { + throw new IllegalStateException("WebRTCWrapper hasn't been closed properly"); } }