jsip
  1. jsip
  2. JSIP-360

TCP race condition to close property the socket

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Patch from Jean for sipx-stable branch:

      Index: src/gov/nist/javax/sip/stack/TCPMessageChannel.java
      ===================================================================
      — src/gov/nist/javax/sip/stack/TCPMessageChannel.java (revision 1848)
      +++ src/gov/nist/javax/sip/stack/TCPMessageChannel.java (working copy)
      @@ -136,7 +136,8 @@
      // Stash away a pointer to our sipStack structure.
      this.sipStack = sipStack;
      this.peerPort = mySock.getPort();
      -
      + this.key = MessageChannel.getKey(peerAddress, peerPort, "TCP");
      +
      this.tcpMessageProcessor = msgProcessor;
      this.myPort = this.tcpMessageProcessor.getPort();
      // Bug report by Vishwashanti Raj Kadiayl
      @@ -183,43 +184,52 @@

      • Close the message channel.
        */
        public void close() {
      • isRunning = false;
      • // we need to close everything because the socket may be closed by the other end
      • // like in LB scenarios sending OPTIONS and killing the socket after it gets the response
      • if (mySock != null) {
      • if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))
      • sipStack.getStackLogger().logDebug("Closing socket " + key);
      • try { - mySock.close(); - }

        catch (IOException ex) {
        + /*
        + * Delay the close of the socket for some time in case it is being used.
        + */
        + sipStack.getTimer().schedule(new TimerTask() {
        +
        + @Override
        + public void run() {
        + isRunning = false;
        + // we need to close everything because the socket may be closed by the other end
        + // like in LB scenarios sending OPTIONS and killing the socket after it gets the response
        + if (mySock != null)

        Unknown macro: {+ if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))+ sipStack.getStackLogger().logDebug("Closing socket " + key);+ try { + mySock.close(); + } catch (IOException ex) { + if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG)) + sipStack.getStackLogger().logDebug("Error closing socket " + ex); + }+ }


        + if(myParser != null)

        { + if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG)) + sipStack.getStackLogger().logDebug("Closing my parser " + myParser); + myParser.close(); + }


        + // no need to close myClientInputStream since myParser.close() above will do it
        + if(myClientOutputStream != null)

        Unknown macro: {+ if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))+ sipStack.getStackLogger().logDebug("Closing client output stream " + myClientOutputStream);+ try { + myClientOutputStream.close(); + } catch (IOException ex) { + if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG)) + sipStack.getStackLogger().logDebug("Error closing client output stream" + ex); + }+ }


        + // remove the "tcp:" part of the key to cleanup the ioHandler hashmap
        + String ioHandlerKey = key.substring(4);
        if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))

      • sipStack.getStackLogger().logDebug("Error closing socket " + ex);
      • }
      • }
      • if(myParser != null) { - if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG)) - sipStack.getStackLogger().logDebug("Closing my parser " + myParser); - myParser.close(); - }
      • // no need to close myClientInputStream since myParser.close() above will do it
      • if(myClientOutputStream != null) {
      • if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))
      • sipStack.getStackLogger().logDebug("Closing client output stream " + myClientOutputStream);
      • try { - myClientOutputStream.close(); - }

        catch (IOException ex)

        { + sipStack.getStackLogger().logDebug("Closing TCP socket " + ioHandlerKey); + // Issue 358 : remove socket and semaphore on close to avoid leaking + sipStack.ioHandler.removeSocket(ioHandlerKey); if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG)) - sipStack.getStackLogger().logDebug("Error closing client output stream" + ex); + sipStack.getStackLogger().logDebug("Closing message Channel " + this); }
      • }
      • // remove the "tcp:" part of the key to cleanup the ioHandler hashmap
      • String ioHandlerKey = key.substring(4);
      • if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))
      • sipStack.getStackLogger().logDebug("Closing TCP socket " + ioHandlerKey);
      • // Issue 358 : remove socket and semaphore on close to avoid leaking
      • sipStack.ioHandler.removeSocket(ioHandlerKey);
      • if (sipStack.isLoggingEnabled(LogWriter.TRACE_DEBUG))
      • sipStack.getStackLogger().logDebug("Closing message Channel " + this);
        + }, 8000);
        }

      /**

        Activity

        Hide
        mranga added a comment -

        Could also be the same issue for TLSMessageChannel. Perhaps the same code (or something close) applies there too.

        Show
        mranga added a comment - Could also be the same issue for TLSMessageChannel. Perhaps the same code (or something close) applies there too.
        Hide
        vralev added a comment -

        Yes it is probably also affected. I will sync them up and with trunk.

        Show
        vralev added a comment - Yes it is probably also affected. I will sync them up and with trunk.

          People

          • Assignee:
            Unassigned
            Reporter:
            vralev
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: