Qpid
  1. Qpid
  2. QPID-4714

AMQConnection close can leak socket connections if exceptions occur earlier in the process

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.20
    • Fix Version/s: 0.22
    • Component/s: Java Client
    • Labels:
      None

      Description

      If any of the operations throws an exception before the call to _delegate.closeConnection(timeout); then we will leak socket connections.

      I already see this happening when running performance tests, where session/connection close operations are timing out. The number of connections keeps growing.

        Activity

        Rajith Attapattu created issue -
        Hide
        Rajith Attapattu added a comment -

        If we move the close connection method inside a finally block, then we can ensure the TCP connection is closed, even if an exception is thrown.

        +                    finally
        +                    {
        +                        try
        +                        {
        +                            _delegate.closeConnection(timeout);
        +                        }
        +                        catch (Exception e)
        +                        {
        +                            _logger.warn("Error closing underlying protocol connection", e);
        +                        }
        +                    }
        
        Show
        Rajith Attapattu added a comment - If we move the close connection method inside a finally block, then we can ensure the TCP connection is closed, even if an exception is thrown. + finally + { + try + { + _delegate.closeConnection(timeout); + } + catch (Exception e) + { + _logger.warn( "Error closing underlying protocol connection" , e); + } + }
        Robbie Gemmell made changes -
        Field Original Value New Value
        Summary AMQConnection close is leaking socket connections AMQConnection close can leak socket connections if exceptions occur earlier in the process
        Hide
        Robbie Gemmell added a comment -

        Seems reasonable.

        Show
        Robbie Gemmell added a comment - Seems reasonable.
        Hide
        Rajith Attapattu added a comment -

        I have checked in a fix at http://svn.apache.org/r1475810

        Should we include this in 0.22 ?

        Show
        Rajith Attapattu added a comment - I have checked in a fix at http://svn.apache.org/r1475810 Should we include this in 0.22 ?
        Hide
        Justin Ross added a comment -

        Reviewed by Robbie. Approved for 0.22.

        Show
        Justin Ross added a comment - Reviewed by Robbie. Approved for 0.22.
        Hide
        Rajith Attapattu added a comment -

        Applied patch to the 0.22 branch.
        See http://svn.apache.org/r1477705

        Show
        Rajith Attapattu added a comment - Applied patch to the 0.22 branch. See http://svn.apache.org/r1477705
        Rajith Attapattu made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 0.22 [ 12324272 ]
        Fix Version/s Future [ 12315490 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Rajith Attapattu
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development