From 5a860c6a8485233e5c084370741dcc7fd9b64eec Mon Sep 17 00:00:00 2001 From: "Stuart O. Anderson" Date: Fri, 9 Nov 2012 02:28:34 -0500 Subject: [PATCH] Propagate errors from RtpSocket to CallManagers: - Ensure audio system shutdown - Propagate IOException from RtpSocket send() failure - Wrap when re-throwing IOExceptions - If IOException bubbles to CallManager, treat as disconnect rather than failure - ZRTPSocket ignores IOExceptions from RtpSocket, since it has its own retry logic. - When we can't construct the RtpSocket, fail with a disconnected call, rather than a clientFailure(). --- .../redphone/audio/CallAudioManager.java | 13 +++++- .../redphone/call/CallManager.java | 6 ++- .../redphone/call/InitiatingCallManager.java | 7 +++- .../redphone/call/ResponderCallManager.java | 6 ++- .../redphone/crypto/SecureRtpSocket.java | 6 +-- .../crypto/zrtp/ZRTPInitiatorSocket.java | 1 + .../redphone/crypto/zrtp/ZRTPSocket.java | 6 ++- .../redphone/network/RtpAudioReader.java | 3 +- .../redphone/network/RtpAudioSender.java | 3 +- .../redphone/network/RtpSocket.java | 42 +++++++------------ 10 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/org/thoughtcrime/redphone/audio/CallAudioManager.java b/src/org/thoughtcrime/redphone/audio/CallAudioManager.java index bff8207f..3d33765a 100644 --- a/src/org/thoughtcrime/redphone/audio/CallAudioManager.java +++ b/src/org/thoughtcrime/redphone/audio/CallAudioManager.java @@ -30,6 +30,7 @@ import org.thoughtcrime.redphone.profiling.TimeProfiler; import org.thoughtcrime.redphone.ui.ApplicationPreferencesActivity; +import java.io.IOException; import java.util.LinkedList; import java.util.NoSuchElementException; @@ -77,7 +78,16 @@ public CallAudioManager( SecureRtpSocket socket, String codecID, Context context simDrops = ApplicationPreferencesActivity.isSimulateDroppedPackets(context); } - public void run() throws AudioException { + public void run() throws AudioException, IOException { + try { + doRun(); + } finally { + doTerminate(); + } + } + + //TODO(Stuart Anderson): Split this up + private void doRun() throws AudioException, IOException { synchronized(this) { if( callDone ) return; runStarted = true; @@ -180,7 +190,6 @@ public void run() throws AudioException { } } - doTerminate(); } private void doTerminate() { diff --git a/src/org/thoughtcrime/redphone/call/CallManager.java b/src/org/thoughtcrime/redphone/call/CallManager.java index a42b2e52..0456466a 100644 --- a/src/org/thoughtcrime/redphone/call/CallManager.java +++ b/src/org/thoughtcrime/redphone/call/CallManager.java @@ -38,6 +38,7 @@ import org.thoughtcrime.redphone.signaling.SignalingSocket; import org.thoughtcrime.redphone.ui.ApplicationPreferencesActivity; +import java.io.IOException; import java.util.Date; /** @@ -116,6 +117,9 @@ public void run() { } catch (AudioException e) { Log.w("CallManager", e); callStateListener.notifyClientError(e.getClientMessage()); + } catch (IOException e) { + Log.w("CallManager", e); + callStateListener.notifyCallDisconnected(); } } @@ -178,7 +182,7 @@ private void printInitDebug() { } //For loopback operation - public void doLoopback() throws AudioException { + public void doLoopback() throws AudioException, IOException { callAudioManager = new CallAudioManager( null, "SPEEX", context ); callAudioManager.run(); } diff --git a/src/org/thoughtcrime/redphone/call/InitiatingCallManager.java b/src/org/thoughtcrime/redphone/call/InitiatingCallManager.java index 9833356c..fe8b95be 100644 --- a/src/org/thoughtcrime/redphone/call/InitiatingCallManager.java +++ b/src/org/thoughtcrime/redphone/call/InitiatingCallManager.java @@ -19,7 +19,6 @@ import android.content.Context; import android.util.Log; - import org.thoughtcrime.redphone.Release; import org.thoughtcrime.redphone.crypto.SecureRtpSocket; import org.thoughtcrime.redphone.crypto.zrtp.MasterSecret; @@ -36,6 +35,7 @@ import org.thoughtcrime.redphone.ui.ApplicationPreferencesActivity; import java.net.InetSocketAddress; +import java.net.SocketException; /** * Call Manager for the coordination of outgoing calls. It initiates @@ -85,7 +85,7 @@ public void run() { InetSocketAddress remoteAddress = new InetSocketAddress(sessionDescriptor.getFullServerName(), sessionDescriptor.relayPort); - secureSocket = new SecureRtpSocket(new RtpSocket(callStateListener, localPort, remoteAddress)); + secureSocket = new SecureRtpSocket(new RtpSocket(localPort, remoteAddress)); zrtpSocket = new ZRTPInitiatorSocket(secureSocket, zid); @@ -106,6 +106,9 @@ public void run() { } catch (SignalingException se) { Log.w("InitiatingCallManager", se); callStateListener.notifyServerFailure(); + } catch (SocketException e) { + Log.w("InitiatingCallManager", e); + callStateListener.notifyCallDisconnected(); } catch( RuntimeException e ) { Log.e( "InitiatingCallManager", "Died with unhandled exception!"); Log.w( "InitiatingCallManager", e ); diff --git a/src/org/thoughtcrime/redphone/call/ResponderCallManager.java b/src/org/thoughtcrime/redphone/call/ResponderCallManager.java index e627d2db..e788d39a 100644 --- a/src/org/thoughtcrime/redphone/call/ResponderCallManager.java +++ b/src/org/thoughtcrime/redphone/call/ResponderCallManager.java @@ -35,6 +35,7 @@ import org.thoughtcrime.redphone.signaling.SignalingSocket; import java.net.InetSocketAddress; +import java.net.SocketException; /** * CallManager responsible for coordinating incoming calls. @@ -87,7 +88,7 @@ public void run() { InetSocketAddress remoteAddress = new InetSocketAddress(sessionDescriptor.getFullServerName(), sessionDescriptor.relayPort); - secureSocket = new SecureRtpSocket(new RtpSocket(callStateListener, localPort, remoteAddress)); + secureSocket = new SecureRtpSocket(new RtpSocket(localPort, remoteAddress)); zrtpSocket = new ZRTPResponderSocket(secureSocket, zid); callStateListener.notifyConnectingtoInitiator(); @@ -105,6 +106,9 @@ public void run() { } catch (LoginFailedException lfe) { Log.w("ResponderCallManager", lfe); callStateListener.notifyLoginFailed(); + } catch (SocketException e) { + Log.w("ResponderCallManager", e); + callStateListener.notifyCallDisconnected(); } catch( RuntimeException e ) { Log.e( "ResponderCallManager", "Died unhandled with exception!"); Log.w( "ResponderCallManager", e ); diff --git a/src/org/thoughtcrime/redphone/crypto/SecureRtpSocket.java b/src/org/thoughtcrime/redphone/crypto/SecureRtpSocket.java index 4f5aa924..9721829b 100644 --- a/src/org/thoughtcrime/redphone/crypto/SecureRtpSocket.java +++ b/src/org/thoughtcrime/redphone/crypto/SecureRtpSocket.java @@ -70,7 +70,7 @@ private void initializeStreamContexts() { this.outgoingContext = new SecureStream(outgoingCipherKey, outgoingMacKey, outgoingSalt); } - public void send(HandshakePacket packet) { + public void send(HandshakePacket packet) throws IOException { packet.setCRC(); socket.send(packet); } @@ -93,7 +93,7 @@ public void setTimeout(int timeoutMillis) { socket.setTimeout(timeoutMillis); } - public void send(SecureRtpPacket packet) { + public void send(SecureRtpPacket packet) throws IOException { TimeProfiler.startBlock("SRPS:send:updateSeq" ); outgoingContext.updateSequence(packet); TimeProfiler.stopBlock("SRPS:send:updateSeq" ); @@ -108,7 +108,7 @@ public void send(SecureRtpPacket packet) { TimeProfiler.stopBlock("SRPS:send:send" ); } - public SecureRtpPacket receive() { + public SecureRtpPacket receive() throws IOException { TimeProfiler.startBlock( "SecureRedphoneSocket::receive" ); RtpPacket barePacket; barePacket = socket.receive(); diff --git a/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPInitiatorSocket.java b/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPInitiatorSocket.java index 61fc56c5..8d9b7428 100644 --- a/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPInitiatorSocket.java +++ b/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPInitiatorSocket.java @@ -22,6 +22,7 @@ import org.thoughtcrime.redphone.Release; import org.thoughtcrime.redphone.crypto.SecureRtpSocket; +import java.io.IOException; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; diff --git a/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPSocket.java b/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPSocket.java index 6e13ed80..8b52c596 100644 --- a/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPSocket.java +++ b/src/org/thoughtcrime/redphone/crypto/zrtp/ZRTPSocket.java @@ -170,7 +170,11 @@ private void sendPacket(HandshakePacket packet) { if (packet != null) { packet.setSequenceNumber(this.sequence++); - socket.send(packet); + try { + socket.send(packet); + } catch (IOException e) { + Log.w("ZRTPSocket", e); + } } } diff --git a/src/org/thoughtcrime/redphone/network/RtpAudioReader.java b/src/org/thoughtcrime/redphone/network/RtpAudioReader.java index 54db183b..bbb865d9 100644 --- a/src/org/thoughtcrime/redphone/network/RtpAudioReader.java +++ b/src/org/thoughtcrime/redphone/network/RtpAudioReader.java @@ -26,6 +26,7 @@ import org.thoughtcrime.redphone.profiling.PeriodicTimer; import org.thoughtcrime.redphone.profiling.TimeProfiler; +import java.io.IOException; import java.util.List; /** * RtpAudioReader listens to a {@link SecureRtpSocket} and writes the incoming {@link EncodedAudioData} to @@ -51,7 +52,7 @@ public RtpAudioReader(List incomingAudio, private int consecutiveReads = 0; private int totalReads = 0; - public void go() { + public void go() throws IOException { //if( !recvTimer.periodically()) return; SecureRtpPacket inPacket = socket.receive(); diff --git a/src/org/thoughtcrime/redphone/network/RtpAudioSender.java b/src/org/thoughtcrime/redphone/network/RtpAudioSender.java index 69b6ac9c..44d95e5b 100644 --- a/src/org/thoughtcrime/redphone/network/RtpAudioSender.java +++ b/src/org/thoughtcrime/redphone/network/RtpAudioSender.java @@ -26,6 +26,7 @@ import org.thoughtcrime.redphone.profiling.PacketLogger; import org.thoughtcrime.redphone.profiling.StatisticsWatcher; +import java.io.IOException; import java.util.LinkedList; /** @@ -61,7 +62,7 @@ public RtpAudioSender(LinkedList outgoingAudio, - public void go() { + public void go() throws IOException { if( audioQueue.size() < audioChunksPerPacket ) { consecutiveSends = 0; diff --git a/src/org/thoughtcrime/redphone/network/RtpSocket.java b/src/org/thoughtcrime/redphone/network/RtpSocket.java index 5da0369d..b54a3af0 100644 --- a/src/org/thoughtcrime/redphone/network/RtpSocket.java +++ b/src/org/thoughtcrime/redphone/network/RtpSocket.java @@ -36,24 +36,14 @@ * @author Stuart O. Anderson */ public class RtpSocket { - - private final CallStateListener callStateListener; - private final byte [] buf = new byte[4096]; - protected DatagramSocket socket; - - public RtpSocket(CallStateListener callStateListener, int localPort, InetSocketAddress remoteAddress) { - this.callStateListener = callStateListener; + private DatagramSocket socket; - try { - socket = new DatagramSocket(localPort); - socket.setSoTimeout(1); - socket.connect(new InetSocketAddress(remoteAddress.getAddress().getHostAddress(), remoteAddress.getPort())); - Log.d( "RtpSocket", "Connected to: " + remoteAddress.getAddress().getHostAddress() ); - } catch (SocketException e) { - e.printStackTrace(); - // XXX-S It seems like this should do something? - } + public RtpSocket(int localPort, InetSocketAddress remoteAddress) throws SocketException { + socket = new DatagramSocket(localPort); + socket.setSoTimeout(1); + socket.connect(new InetSocketAddress(remoteAddress.getAddress().getHostAddress(), remoteAddress.getPort())); + Log.d( "RtpSocket", "Connected to: " + remoteAddress.getAddress().getHostAddress() ); } public void setTimeout(int timeoutMillis) { @@ -67,12 +57,14 @@ public void setTimeout(int timeoutMillis) { private long totalSendTime = 0; private PeriodicTimer pt = new PeriodicTimer(10000); - public void send(RtpPacket outPacket) { + public void send(RtpPacket outPacket) throws IOException { long start = SystemClock.uptimeMillis(); try { socket.send(new DatagramPacket(outPacket.getPacket(), outPacket.getPacketLength())); } catch (IOException e) { - Log.w("RtpSocket", e); + if (!socket.isClosed()) { + throw new IOException(e); + } } long stop = SystemClock.uptimeMillis(); totalSendTime += stop - start; @@ -82,7 +74,7 @@ public void send(RtpPacket outPacket) { } } - public RtpPacket receive() { + public RtpPacket receive() throws IOException { try { DatagramPacket dataPack = new DatagramPacket(buf, buf.length); socket.setSoTimeout(1); @@ -90,15 +82,11 @@ public RtpPacket receive() { RtpPacket inPacket = new RtpPacket(dataPack.getData(), dataPack.getLength()); return inPacket; } catch( SocketTimeoutException e ) { - return null; - } catch (SocketException e) { - // XXX-S I don't think this should be reaching back up the stack from this level. - // Instead, it seems like this should be throwing IOException/SocketException up - // the stack to CallAudioManager or wherever, which should handle reaching back up - // from that point in the stack. - callStateListener.notifyCallDisconnected(); + //Do Nothing. } catch (IOException e) { - callStateListener.notifyCallDisconnected(); + if (!socket.isClosed()) { + throw new IOException(e); + } } return null; }