Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-5509

Potential connection leaks caused by the connectivity check

    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 0.14.0, 0.15.0, 0.14.1, 0.14.2
    • 0.16.0
    • Go - Library
    • None

    Description

      Historically, for a TTransport, IsOpen returns false means that the connection is already explicitly closed. So third party code (for example, client pool management logic) could just throw the connection away after IsOpen returned false without worry.

      But since the adding of connectivity check in go library (first release version 0.14.0), that is no longer true: IsOpen could return true when the remote closed the connection, and in such cases Close shall still be explicitly called. If we just throw the connection away without explicitly calling Close, the action of "throw away the connection" will cause connection leaks.

      There are 2 ways to mitigate it:

      1. Document in IsOpen that IsOpen returning false does not necessarily mean the connection is already closed, and an explicit Close call is still needed.
      2. Implicitly call Close inside IsOpen when connectivity check fails. This way we keep the assumption that IsOpen returns false means Close was already called being true.

      dcelasun what's your preference between the 2 paths?

      Attachments

        Issue Links

          Activity

            People

              fishywang Yuxuan Wang
              fishywang Yuxuan Wang
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 2h 10m
                  2h 10m