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, 6.0
    • 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.0 kB
        Mark Miller
      2. SOLR-4327.patch
        1 kB
        Mark Miller

        Issue Links

          Activity

          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.
          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.
          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 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
          Mark Miller added a comment -

          Thanks again Karl!

          Show
          Mark Miller added a comment - Thanks again Karl!
          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) {}
          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
          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
          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 -

          My mistake for not taking the time to really dig into this one. This was a mistake to add, though it had no ill affect. I've addressed it in SOLR-8451 and added some connection reuse testing.

          Show
          Mark Miller added a comment - My mistake for not taking the time to really dig into this one. This was a mistake to add, though it had no ill affect. I've addressed it in SOLR-8451 and added some connection reuse testing.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development