Lucene - Core
  1. Lucene - Core
  2. LUCENE-3326

MoreLikeThis reuses a reader after it has already closed it

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 3.4, 4.0-ALPHA
    • Component/s: modules/other
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      MoreLikeThis has a fatal bug whereby it tries to reuse a reader for multiple fields:

          Map<String,Int> words = new HashMap<String,Int>();
          for (int i = 0; i < fieldNames.length; i++) {
              String fieldName = fieldNames[i];
              addTermFrequencies(r, words, fieldName);
          }
      

      However, addTermFrequencies() is creating a TokenStream for this reader:

          TokenStream ts = analyzer.reusableTokenStream(fieldName, r);
          int tokenCount=0;
          // for every token
          CharTermAttribute termAtt = ts.addAttribute(CharTermAttribute.class);
          ts.reset();
          while (ts.incrementToken()) {
              /* body omitted */
          }
          ts.end();
          ts.close();
      

      When it closes this analyser, it closes the underlying reader. Then the second time around the loop, you get:

      Caused by: java.io.IOException: Stream closed
      	at sun.nio.cs.StreamDecoder.ensureOpen(StreamDecoder.java:27)
      	at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:128)
      	at java.io.InputStreamReader.read(InputStreamReader.java:167)
      	at com.acme.util.CompositeReader.read(CompositeReader.java:101)
      	at org.apache.lucene.analysis.standard.StandardTokenizerImpl.zzRefill(StandardTokenizerImpl.java:803)
      	at org.apache.lucene.analysis.standard.StandardTokenizerImpl.getNextToken(StandardTokenizerImpl.java:1010)
      	at org.apache.lucene.analysis.standard.StandardTokenizer.incrementToken(StandardTokenizer.java:178)
      	at org.apache.lucene.analysis.standard.StandardFilter.incrementTokenClassic(StandardFilter.java:61)
      	at org.apache.lucene.analysis.standard.StandardFilter.incrementToken(StandardFilter.java:57)
      	at com.acme.storage.index.analyser.NormaliseFilter.incrementToken(NormaliseFilter.java:51)
      	at org.apache.lucene.analysis.LowerCaseFilter.incrementToken(LowerCaseFilter.java:60)
      	at org.apache.lucene.search.similar.MoreLikeThis.addTermFrequencies(MoreLikeThis.java:931)
      	at org.apache.lucene.search.similar.MoreLikeThis.retrieveTerms(MoreLikeThis.java:1003)
      	at org.apache.lucene.search.similar.MoreLikeThis.retrieveInterestingTerms(MoreLikeThis.java:1036)
      

      My first thought was that it seems like a "ReaderFactory" of sorts should be passed in so that a new Reader can be created for the second field (maybe the factory could be passed the field name, so that if someone wanted to pass a different reader to each, they could.)

      Interestingly, the methods taking File and URL exhibit the same issue. I'm not sure what to do about those (and we're not using them.) The method taking File could open the file twice, but the method taking a URL probably shouldn't fetch the same URL twice.

        Activity

        Hide
        Robert Muir added a comment -

        the logic of this class is broken: the field parameter taken here is just to specify the fieldname passed to the Analyzer when analyzing the tokens (e.g. some analyzers behave differently depending upon field).

        There should be no loop across fields... instead you should be forced to provide this fieldname as an argument if you pass in a reader (analyze this content with my analyzer using field X)

        As far as I can tell, this has been broken for a long time: if your analyzer works the same way across all fields you would previously never notice a problem, because it would analyze the text with the "first" one, but didnt close the reader, passing an exhausted reader across other field names.

        Show
        Robert Muir added a comment - the logic of this class is broken: the field parameter taken here is just to specify the fieldname passed to the Analyzer when analyzing the tokens (e.g. some analyzers behave differently depending upon field). There should be no loop across fields... instead you should be forced to provide this fieldname as an argument if you pass in a reader (analyze this content with my analyzer using field X) As far as I can tell, this has been broken for a long time: if your analyzer works the same way across all fields you would previously never notice a problem, because it would analyze the text with the "first" one, but didnt close the reader, passing an exhausted reader across other field names.
        Hide
        Robert Muir added a comment -

        here is a patch, that requires fieldname.

        places using this Reader method before such as Solr and xml-query-parser just pass field[0], to get the old effect of the non-reader-closing version, but really they should separately have configurable what field is passed to the analyzer for analysis.

        also i nuked these File, URL methods, because for example the MoreLikeThisQuery was using default charset!

        Show
        Robert Muir added a comment - here is a patch, that requires fieldname. places using this Reader method before such as Solr and xml-query-parser just pass field [0] , to get the old effect of the non-reader-closing version, but really they should separately have configurable what field is passed to the analyzer for analysis. also i nuked these File, URL methods, because for example the MoreLikeThisQuery was using default charset!
        Hide
        Uwe Schindler added a comment -

        +1 nuke default charset shit!

        Show
        Uwe Schindler added a comment - +1 nuke default charset shit!
        Hide
        Robert Muir added a comment -

        thanks for reporting this Trejkaz!

        Show
        Robert Muir added a comment - thanks for reporting this Trejkaz!
        Hide
        Carl Austin added a comment -

        In case you were unaware (as the JIRA says affects 3.3) this also affects 3.2 as I have just reproduced it.
        Thanks.

        Show
        Carl Austin added a comment - In case you were unaware (as the JIRA says affects 3.3) this also affects 3.2 as I have just reproduced it. Thanks.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development