You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2019/03/29 13:04:37 UTC

[lucene-solr] branch branch_7_7 updated: LUCENE-8726: ValueSource.asDoubleValuesSource() could leak a reference to IndexSearcher

This is an automated email from the ASF dual-hosted git repository.

romseygeek pushed a commit to branch branch_7_7
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_7_7 by this push:
     new 2761bb5  LUCENE-8726: ValueSource.asDoubleValuesSource() could leak a reference to IndexSearcher
2761bb5 is described below

commit 2761bb510807f5b8949b641c8cc243c372ab8f0f
Author: Alan Woodward <ro...@apache.org>
AuthorDate: Thu Mar 14 10:17:12 2019 +0000

    LUCENE-8726: ValueSource.asDoubleValuesSource() could leak a reference to IndexSearcher
---
 lucene/CHANGES.txt                                 |  9 +++-
 .../apache/lucene/search/DoubleValuesSource.java   |  3 ++
 .../queries/function/FunctionScoreQuery.java       | 11 ++++-
 .../lucene/queries/function/ValueSource.java       | 14 +++---
 .../lucene/queries/function/TestValueSources.java  | 51 +++++++---------------
 5 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index d1b9252..b5241be 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -3,16 +3,21 @@ Lucene Change Log
 For more information on past and future Lucene versions, please see:
 http://s.apache.org/luceneversions
 
+======================= Lucene 7.7.2 =======================
 
-======================= Lucene 7.7.1 =======================
+Bug fixes
 
-Bug fixes:
+* LUCENE-8726: ValueSource.asDoubleValuesSource() could leak a reference to
+  IndexSearcher (Alan Woodward, Yury Pakhomov)
 
 * LUCENE-8735: FilterDirectory.getPendingDeletions now forwards to the delegate
   even the method is not abstract in the super class. This prevents issues
   where our best effort in carrying on generations in the IndexWriter since pending
   deletions are swallowed by the FilterDirectory. (Henning Andersen, Simon Willnauer)
 
+======================= Lucene 7.7.1 =======================
+(No changes)
+
 ======================= Lucene 7.7.0 =======================
 
 Changes in Runtime Behavior
diff --git a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
index bdcd068..ffe9b7b 100644
--- a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
+++ b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
@@ -83,6 +83,9 @@ public abstract class DoubleValuesSource implements SegmentCacheable {
    * Queries that use DoubleValuesSource objects should call rewrite() during
    * {@link Query#createWeight(IndexSearcher, boolean, float)} rather than during
    * {@link Query#rewrite(IndexReader)} to avoid IndexReader reference leakage
+   *
+   * For the same reason, implementations that cache references to the IndexSearcher
+   * should return a new object from this method.
    */
   public abstract DoubleValuesSource rewrite(IndexSearcher reader) throws IOException;
 
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java b/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java
index 9e35c99..420baab 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java
@@ -64,6 +64,13 @@ public final class FunctionScoreQuery extends Query {
   }
 
   /**
+   * @return the modifying DoubleValuesSource
+   */
+  public DoubleValuesSource getSource() {
+    return source;
+  }
+
+  /**
    * Returns a FunctionScoreQuery where the scores of a wrapped query are multiplied by
    * the value of a DoubleValuesSource.
    *
@@ -188,9 +195,9 @@ public final class FunctionScoreQuery extends Query {
 
   }
 
-  private static class MultiplicativeBoostValuesSource extends DoubleValuesSource {
+  static class MultiplicativeBoostValuesSource extends DoubleValuesSource {
 
-    private final DoubleValuesSource boost;
+    final DoubleValuesSource boost;
 
     private MultiplicativeBoostValuesSource(DoubleValuesSource boost) {
       this.boost = boost;
diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
index bab02ad..0c65546 100644
--- a/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
+++ b/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
@@ -191,16 +191,17 @@ public abstract class ValueSource {
    * Expose this ValueSource as a DoubleValuesSource
    */
   public DoubleValuesSource asDoubleValuesSource() {
-    return new WrappedDoubleValuesSource(this);
+    return new WrappedDoubleValuesSource(this, null);
   }
 
-  private static class WrappedDoubleValuesSource extends DoubleValuesSource {
+  static class WrappedDoubleValuesSource extends DoubleValuesSource {
 
-    private final ValueSource in;
-    private IndexSearcher searcher;
+    final ValueSource in;
+    final IndexSearcher searcher;
 
-    private WrappedDoubleValuesSource(ValueSource in) {
+    private WrappedDoubleValuesSource(ValueSource in, IndexSearcher searcher) {
       this.in = in;
+      this.searcher = searcher;
     }
 
     @Override
@@ -253,8 +254,7 @@ public abstract class ValueSource {
 
     @Override
     public DoubleValuesSource rewrite(IndexSearcher searcher) throws IOException {
-      this.searcher = searcher;
-      return this;
+      return new WrappedDoubleValuesSource(in, searcher);
     }
 
     @Override
diff --git a/lucene/queries/src/test/org/apache/lucene/queries/function/TestValueSources.java b/lucene/queries/src/test/org/apache/lucene/queries/function/TestValueSources.java
index 4822e03..3ebb5fe 100644
--- a/lucene/queries/src/test/org/apache/lucene/queries/function/TestValueSources.java
+++ b/lucene/queries/src/test/org/apache/lucene/queries/function/TestValueSources.java
@@ -36,42 +36,7 @@ import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.RandomIndexWriter;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.queries.function.docvalues.FloatDocValues;
-import org.apache.lucene.queries.function.valuesource.BytesRefFieldSource;
-import org.apache.lucene.queries.function.valuesource.ConstValueSource;
-import org.apache.lucene.queries.function.valuesource.DivFloatFunction;
-import org.apache.lucene.queries.function.valuesource.DocFreqValueSource;
-import org.apache.lucene.queries.function.valuesource.DoubleConstValueSource;
-import org.apache.lucene.queries.function.valuesource.DoubleFieldSource;
-import org.apache.lucene.queries.function.valuesource.FloatFieldSource;
-import org.apache.lucene.queries.function.valuesource.IDFValueSource;
-import org.apache.lucene.queries.function.valuesource.IfFunction;
-import org.apache.lucene.queries.function.valuesource.IntFieldSource;
-import org.apache.lucene.queries.function.valuesource.JoinDocFreqValueSource;
-import org.apache.lucene.queries.function.valuesource.LinearFloatFunction;
-import org.apache.lucene.queries.function.valuesource.LiteralValueSource;
-import org.apache.lucene.queries.function.valuesource.LongFieldSource;
-import org.apache.lucene.queries.function.valuesource.MaxDocValueSource;
-import org.apache.lucene.queries.function.valuesource.MaxFloatFunction;
-import org.apache.lucene.queries.function.valuesource.MinFloatFunction;
-import org.apache.lucene.queries.function.valuesource.MultiFloatFunction;
-import org.apache.lucene.queries.function.valuesource.MultiFunction;
-import org.apache.lucene.queries.function.valuesource.MultiValuedDoubleFieldSource;
-import org.apache.lucene.queries.function.valuesource.MultiValuedFloatFieldSource;
-import org.apache.lucene.queries.function.valuesource.MultiValuedIntFieldSource;
-import org.apache.lucene.queries.function.valuesource.MultiValuedLongFieldSource;
-import org.apache.lucene.queries.function.valuesource.NormValueSource;
-import org.apache.lucene.queries.function.valuesource.NumDocsValueSource;
-import org.apache.lucene.queries.function.valuesource.PowFloatFunction;
-import org.apache.lucene.queries.function.valuesource.ProductFloatFunction;
-import org.apache.lucene.queries.function.valuesource.QueryValueSource;
-import org.apache.lucene.queries.function.valuesource.RangeMapFloatFunction;
-import org.apache.lucene.queries.function.valuesource.ReciprocalFloatFunction;
-import org.apache.lucene.queries.function.valuesource.ScaleFloatFunction;
-import org.apache.lucene.queries.function.valuesource.SumFloatFunction;
-import org.apache.lucene.queries.function.valuesource.SumTotalTermFreqValueSource;
-import org.apache.lucene.queries.function.valuesource.TFValueSource;
-import org.apache.lucene.queries.function.valuesource.TermFreqValueSource;
-import org.apache.lucene.queries.function.valuesource.TotalTermFreqValueSource;
+import org.apache.lucene.queries.function.valuesource.*;
 import org.apache.lucene.search.CheckHits;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Query;
@@ -600,6 +565,20 @@ public class TestValueSources extends LuceneTestCase {
       }
     }
   }
+
+  public void testWrappingAsDoubleValues() throws IOException {
+
+    FunctionScoreQuery q = FunctionScoreQuery.boostByValue(new TermQuery(new Term("f", "t")),
+        new DoubleFieldSource("double").asDoubleValuesSource());
+
+    searcher.createWeight(searcher.rewrite(q), true, 1);
+
+    // assert that the query has not cached a reference to the IndexSearcher
+    FunctionScoreQuery.MultiplicativeBoostValuesSource source1 = (FunctionScoreQuery.MultiplicativeBoostValuesSource) q.getSource();
+    ValueSource.WrappedDoubleValuesSource source2 = (ValueSource.WrappedDoubleValuesSource) source1.boost;
+    assertNull(source2.searcher);
+
+  }
     
   /** 
    * Asserts that for every doc, the {@link FunctionValues#exists} value