Solr
  1. Solr
  2. SOLR-4327

SolrJ code review indicates potential for leaked HttpClient connections

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.5.1, 4.6, Trunk
    • Component/s: None
    • Labels:
      None

      Description

      The SolrJ HttpSolrServer implementation does not seem to handle errors properly and seems capable of leaking HttpClient connections. See the request() method in org.apache.solr.client.solrj.impl.HttpSolrServer. The issue is that exceptions thrown from within this method do not necessarily consume the stream when an exception is thrown. There is a try/finally block which reads (in part):

          } finally {
            if (respBody != null && processor!=null) {
              try {
                respBody.close();
              } catch (Throwable t) {} // ignore
            }
          }
      

      But, in order to always guarantee consumption of the stream, it should include:

      method.abort();
      
      1. SOLR-4327.patch
        1 kB
        Mark Miller
      2. SOLR-4327.patch
        1.0 kB
        Mark Miller

        Activity

        Hide
        Mark Miller added a comment -

        See SOLR-4992 if you are interested in the catching Throwable issue.

        Show
        Mark Miller added a comment - See SOLR-4992 if you are interested in the catching Throwable issue.
        Hide
        Mark Miller added a comment -

        That change has nothing to do with this JIRA issue.

        Show
        Mark Miller added a comment - That change has nothing to do with this JIRA issue.
        Hide
        David Smiley added a comment -

        +1 Oleg. Mark, please change to catch Exception.

        Show
        David Smiley added a comment - +1 Oleg. Mark, please change to catch Exception.
        Hide
        Oleg Kalnichevski added a comment -

        This is none of my business, but I think and catching and ignoring Throwables is an exceptionally bad idea

        try {
            respBody.close();
        } catch (Throwable t) {} // ignore
        

        At the very least it should be

        try {
            respBody.close();
        } catch (Exception ignore) {}
        
        Show
        Oleg Kalnichevski added a comment - This is none of my business, but I think and catching and ignoring Throwables is an exceptionally bad idea try { respBody.close(); } catch (Throwable t) {} // ignore At the very least it should be try { respBody.close(); } catch (Exception ignore) {}
        Mark Miller made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Mark Miller added a comment -

        Thanks again Karl!

        Show
        Mark Miller added a comment - Thanks again Karl!
        Hide
        ASF subversion and git services added a comment -

        Commit 1531598 from Mark Miller in branch 'dev/branches/lucene_solr_4_5'
        [ https://svn.apache.org/r1531598 ]

        SOLR-4327: HttpSolrServer can leak connections on errors.

        Show
        ASF subversion and git services added a comment - Commit 1531598 from Mark Miller in branch 'dev/branches/lucene_solr_4_5' [ https://svn.apache.org/r1531598 ] SOLR-4327 : HttpSolrServer can leak connections on errors.
        Hide
        ASF subversion and git services added a comment -

        Commit 1531597 from Mark Miller in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1531597 ]

        SOLR-4327: HttpSolrServer can leak connections on errors.

        Show
        ASF subversion and git services added a comment - Commit 1531597 from Mark Miller in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1531597 ] SOLR-4327 : HttpSolrServer can leak connections on errors.
        Hide
        ASF subversion and git services added a comment -

        Commit 1531596 from Mark Miller in branch 'dev/trunk'
        [ https://svn.apache.org/r1531596 ]

        SOLR-4327: HttpSolrServer can leak connections on errors.

        Show
        ASF subversion and git services added a comment - Commit 1531596 from Mark Miller in branch 'dev/trunk' [ https://svn.apache.org/r1531596 ] SOLR-4327 : HttpSolrServer can leak connections on errors.
        Mark Miller made changes -
        Attachment SOLR-4327.patch [ 12608175 ]
        Hide
        Mark Miller added a comment -

        Thanks Karl - this one missed my radar. I'll commit shortly.

        Show
        Mark Miller added a comment - Thanks Karl - this one missed my radar. I'll commit shortly.
        Mark Miller made changes -
        Attachment SOLR-4327.patch [ 12608170 ]
        Mark Miller made changes -
        Field Original Value New Value
        Assignee Mark Miller [ markrmiller@gmail.com ]
        Fix Version/s 4.5.1 [ 12325264 ]
        Fix Version/s 4.6 [ 12325000 ]
        Fix Version/s 5.0 [ 12321664 ]
        Karl Wright created issue -

          People

          • Assignee:
            Mark Miller
            Reporter:
            Karl Wright
          • Votes:
            3 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development