From ff9ff45bf838e64b6d1f6947af29db1c08cdb161 Mon Sep 17 00:00:00 2001 From: fre42 Date: Fri, 9 Feb 2018 16:53:49 +0100 Subject: [PATCH] fixed race condition: SocketException due to mysock closed while sending response --- .../javax/sip/stack/TCPMessageChannel.java | 41 +++++++++++-------- .../javax/sip/stack/TLSMessageChannel.java | 20 +++++---- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/gov/nist/javax/sip/stack/TCPMessageChannel.java b/src/gov/nist/javax/sip/stack/TCPMessageChannel.java index a97989943..fb1fc38a0 100755 --- a/src/gov/nist/javax/sip/stack/TCPMessageChannel.java +++ b/src/gov/nist/javax/sip/stack/TCPMessageChannel.java @@ -235,6 +235,9 @@ protected synchronized void sendMessage(byte[] msg, boolean isClient) throws IO Socket sock = null; IOException problem = null; + // try to prevent at least the worst thread safety issues by using a local variable ... + Socket mySockLocal = mySock; + try { sock = this.sipStack.ioHandler.sendBytes(this.messageProcessor.getIpAddress(), this.peerAddress, this.peerPort, this.peerProtocol, msg, isClient, this); @@ -270,16 +273,16 @@ protected synchronized void sendMessage(byte[] msg, boolean isClient) throws IO // if (mySock == null && s != null) { // this.uncache(); // } else - if (sock != mySock && sock != null) { - if (mySock != null) { + if (sock != mySockLocal && sock != null) { + if (mySockLocal != null) { if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { logger.logWarning( "Old socket different than new socket on channel " + key); logger.logStackTrace(); logger.logWarning( - "Old socket local ip address " + mySock.getLocalSocketAddress()); + "Old socket local ip address " + mySockLocal.getLocalSocketAddress()); logger.logWarning( - "Old socket remote ip address " + mySock.getRemoteSocketAddress()); + "Old socket remote ip address " + mySockLocal.getRemoteSocketAddress()); logger.logWarning( "New socket local ip address " + sock.getLocalSocketAddress()); logger.logWarning( @@ -288,19 +291,23 @@ protected synchronized void sendMessage(byte[] msg, boolean isClient) throws IO close(false, false); } if(problem == null) { - if(mySock != null) { - if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { - logger.logWarning( - "There was no exception for the retry mechanism so creating a new thread based on the new socket for incoming " + key); - } - } - mySock = sock; - this.myClientInputStream = mySock.getInputStream(); - this.myClientOutputStream = mySock.getOutputStream(); - Thread thread = new Thread(this); - thread.setDaemon(true); - thread.setName("TCPMessageChannelThread"); - thread.start(); + if (mySockLocal != null) { + if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { + logger.logWarning( + "There was no exception for the retry mechanism so creating a new thread based on the new socket for incoming " + key); + } + } + // NOTE: need to consider refactoring the whole socket handling with respect to thread safety + if (mySockLocal == mySock) { + // still not thread safe :-( but what else to do? + mySock = sock; + this.myClientInputStream = mySock.getInputStream(); + this.myClientOutputStream = mySock.getOutputStream(); + Thread thread = new Thread(this); + thread.setDaemon(true); + thread.setName("TCPMessageChannelThread"); + thread.start(); + } } else { if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { logger.logWarning( diff --git a/src/gov/nist/javax/sip/stack/TLSMessageChannel.java b/src/gov/nist/javax/sip/stack/TLSMessageChannel.java index 007d09401..534a08c0a 100755 --- a/src/gov/nist/javax/sip/stack/TLSMessageChannel.java +++ b/src/gov/nist/javax/sip/stack/TLSMessageChannel.java @@ -234,6 +234,8 @@ protected synchronized void sendMessage(byte[] msg, boolean retry) throws IOExce } Socket sock = null; IOException problem = null; + // try to prevent at least the worst thread safety issues by using a local variable ... + Socket mySockLocal = mySock; try { sock = this.sipStack.ioHandler.sendBytes( this.getMessageProcessor().getIpAddress(), this.peerAddress, this.peerPort, @@ -278,18 +280,22 @@ protected synchronized void sendMessage(byte[] msg, boolean retry) throws IOExce close(false, false); } if(problem == null) { - if(mySock != null) { + if (mySockLocal != null) { if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { logger.logWarning( "There was no exception for the retry mechanism so creating a new thread based on the new socket for incoming " + key); } } - mySock = sock; - this.myClientInputStream = mySock.getInputStream(); - Thread thread = new Thread(this); - thread.setDaemon(true); - thread.setName("TCPMessageChannelThread"); - thread.start(); + // NOTE: need to consider refactoring the whole socket handling with respect to thread safety + if (mySockLocal == mySock) { + // still not thread safe :-( but what else to do? + mySock = sock; + this.myClientInputStream = mySock.getInputStream(); + Thread thread = new Thread(this); + thread.setDaemon(true); + thread.setName("TCPMessageChannelThread"); + thread.start(); + } } else { if(logger.isLoggingEnabled(LogWriter.TRACE_WARN)) { logger.logWarning(