From d19dcb4ff0c221400b94705c3d0631c274fd6e75 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 14 Mar 2019 10:17:12 +0000 Subject: [PATCH] LUCENE-8726: ValueSource.asDoubleValuesSource() could leak a reference to IndexSearcher --- .../lucene/search/DoubleValuesSource.java | 5 +- .../queries/function/FunctionScoreQuery.java | 4 +- .../lucene/queries/function/ValueSource.java | 14 ++--- .../queries/function/TestValueSources.java | 52 ++++++------------- 4 files changed, 29 insertions(+), 46 deletions(-) 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 929e45f2a35..6446f7f3e14 100644 --- a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java +++ b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java @@ -82,7 +82,10 @@ public abstract class DoubleValuesSource implements SegmentCacheable { * * Queries that use DoubleValuesSource objects should call rewrite() during * {@link Query#createWeight(IndexSearcher, ScoreMode, float)} rather than during - * {@link Query#rewrite(IndexReader)} to avoid IndexReader reference leakage + * {@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 5268a430a3a..29d6dd9f414 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 @@ -235,9 +235,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 221e822d1f1..0cbfe89a46d 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 @@ -182,16 +182,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 @@ -247,8 +248,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 1d7fefbbab8..0b84c93f2b5 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,46 +36,12 @@ 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; import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.ScoreMode; import org.apache.lucene.search.Sort; import org.apache.lucene.search.SortField; import org.apache.lucene.search.SortedNumericSelector.Type; @@ -600,6 +566,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), ScoreMode.COMPLETE, 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