LUCENE-5532: AutomatonQuery.hashCode is not thread safe

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1578483 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -70,6 +70,10 @@ Changes in Runtime Behavior
   behavior of silently ignoring these terms, use LengthFilter in your Analyzer.
   (hossman, Mike McCandless, Varun Thacker)
 
+* LUCENE-5532: AutomatonQuery.equals is no longer implemented as "accepts same language".
+  This was inconsistent with hashCode, and unnecessary for any subclasses in Lucene.
+  If you desire this in a custom subclass, minimize the automaton.  (Robert Muir)
+
 New Features
 
 * LUCENE-5454: Add SortedSetSortField to lucene/sandbox, to allow sorting
@@ -198,6 +202,8 @@ Bug fixes
   facets through DrillSideways, for example.  (Jose Peleteiro, Mike
   McCandless)
 
+* LUCENE-5532: AutomatonQuery.hashCode was not thread-safe. (Robert Muir)
+
 Test Framework
 
 * LUCENE-5449: Rename _TestUtil and _TestHelper to remove the leading _.

lucene/core/src/java/org/apache/lucene/search/AutomatonQuery.java

@@ -25,7 +25,6 @@ import org.apache.lucene.index.TermsEnum;
 import org.apache.lucene.util.AttributeSource;
 import org.apache.lucene.util.ToStringUtils;
 import org.apache.lucene.util.automaton.Automaton;
-import org.apache.lucene.util.automaton.BasicOperations;
 import org.apache.lucene.util.automaton.CompiledAutomaton;
 
 /**
@@ -77,16 +76,7 @@ public class AutomatonQuery extends MultiTermQuery {
   public int hashCode() {
     final int prime = 31;
     int result = super.hashCode();
-    if (automaton != null) {
-      // we already minimized the automaton in the ctor, so
-      // this hash code will be the same for automata that
-      // are the same:
-      int automatonHashCode = automaton.getNumberOfStates() * 3 + automaton.getNumberOfTransitions() * 2;
-      if (automatonHashCode == 0) {
-        automatonHashCode = 1;
-      }
-      result = prime * result + automatonHashCode;
-    }
+    result = prime * result + compiled.hashCode();
     result = prime * result + ((term == null) ? 0 : term.hashCode());
     return result;
   }
@@ -100,10 +90,7 @@ public class AutomatonQuery extends MultiTermQuery {
     if (getClass() != obj.getClass())
       return false;
     AutomatonQuery other = (AutomatonQuery) obj;
-    if (automaton == null) {
-      if (other.automaton != null)
-        return false;
-    } else if (!BasicOperations.sameLanguage(automaton, other.automaton))
+    if (!compiled.equals(other.compiled))
       return false;
     if (term == null) {
       if (other.term != null)

lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java

@@ -271,12 +271,12 @@ public class Automaton implements Cloneable {
       expandSingleton();
       final Set<State> visited = new HashSet<>();
       final LinkedList<State> worklist = new LinkedList<>();
-      numberedStates = new State[4];
+      State states[] = new State[4];
       int upto = 0;
       worklist.add(initial);
       visited.add(initial);
       initial.number = upto;
-      numberedStates[upto] = initial;
+      states[upto] = initial;
       upto++;
       while (worklist.size() > 0) {
         State s = worklist.removeFirst();
@@ -286,21 +286,22 @@ public class Automaton implements Cloneable {
             visited.add(t.to);
             worklist.add(t.to);
             t.to.number = upto;
-            if (upto == numberedStates.length) {
+            if (upto == states.length) {
               final State[] newArray = new State[ArrayUtil.oversize(1+upto, RamUsageEstimator.NUM_BYTES_OBJECT_REF)];
-              System.arraycopy(numberedStates, 0, newArray, 0, upto);
-              numberedStates = newArray;
+              System.arraycopy(states, 0, newArray, 0, upto);
+              states = newArray;
             }
-            numberedStates[upto] = t.to;
+            states[upto] = t.to;
             upto++;
           }
         }
       }
-      if (numberedStates.length != upto) {
+      if (states.length != upto) {
         final State[] newArray = new State[upto];
-        System.arraycopy(numberedStates, 0, newArray, 0, upto);
-        numberedStates = newArray;
+        System.arraycopy(states, 0, newArray, 0, upto);
+        states = newArray;
       }
+      numberedStates = states;
     }
 
     return numberedStates;

lucene/core/src/java/org/apache/lucene/util/automaton/CompiledAutomaton.java

@@ -19,6 +19,7 @@ package org.apache.lucene.util.automaton;
   
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.lucene.index.Terms;
@@ -365,4 +366,30 @@ public class CompiledAutomaton {
     }
     return b.append("}\n").toString();
   }
+
+  @Override
+  public int hashCode() {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + ((runAutomaton == null) ? 0 : runAutomaton.hashCode());
+    result = prime * result + ((term == null) ? 0 : term.hashCode());
+    result = prime * result + ((type == null) ? 0 : type.hashCode());
+    return result;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) return true;
+    if (obj == null) return false;
+    if (getClass() != obj.getClass()) return false;
+    CompiledAutomaton other = (CompiledAutomaton) obj;
+    if (type != other.type) return false;
+    if (type == AUTOMATON_TYPE.SINGLE || type == AUTOMATON_TYPE.PREFIX) {
+      if (!term.equals(other.term)) return false;
+    } else if (type == AUTOMATON_TYPE.NORMAL) {
+      if (!runAutomaton.equals(other.runAutomaton)) return false;
+    }
+
+    return true;
+  }
 }

lucene/core/src/java/org/apache/lucene/util/automaton/RunAutomaton.java

@@ -29,6 +29,8 @@
 
 package org.apache.lucene.util.automaton;
 
+import java.util.Arrays;
+
 /**
  * Finite-state automaton with fast run operation.
  * 
@@ -165,4 +167,30 @@ public abstract class RunAutomaton {
     else
       return transitions[state * points.length + classmap[c]];
   }
+
+  @Override
+  public int hashCode() {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + initial;
+    result = prime * result + maxInterval;
+    result = prime * result + points.length;
+    result = prime * result + size;
+    return result;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (this == obj) return true;
+    if (obj == null) return false;
+    if (getClass() != obj.getClass()) return false;
+    RunAutomaton other = (RunAutomaton) obj;
+    if (initial != other.initial) return false;
+    if (maxInterval != other.maxInterval) return false;
+    if (size != other.size) return false;
+    if (!Arrays.equals(points, other.points)) return false;
+    if (!Arrays.equals(accept, other.accept)) return false;
+    if (!Arrays.equals(transitions, other.transitions)) return false;
+    return true;
+  }
 }

lucene/core/src/test/org/apache/lucene/search/TestAutomatonQuery.java

@@ -18,6 +18,7 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
@@ -30,7 +31,10 @@ import org.apache.lucene.index.Terms;
 import org.apache.lucene.index.TermsEnum;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.Rethrow;
+import org.apache.lucene.util.TestUtil;
 import org.apache.lucene.util.automaton.Automaton;
+import org.apache.lucene.util.automaton.AutomatonTestUtil;
 import org.apache.lucene.util.automaton.BasicAutomata;
 import org.apache.lucene.util.automaton.BasicOperations;
 
@@ -148,8 +152,10 @@ public class TestAutomatonQuery extends LuceneTestCase {
     AutomatonQuery a5 = new AutomatonQuery(newTerm("blah"), BasicAutomata
         .makeString("foobar"));
     
+    assertEquals(a1.hashCode(), a2.hashCode());
     assertEquals(a1, a2);
     
+    assertEquals(a1.hashCode(), a3.hashCode());
     assertEquals(a1, a3);
   
     // different class
@@ -204,4 +210,35 @@ public class TestAutomatonQuery extends LuceneTestCase {
     assertSame(TermsEnum.EMPTY, aq.getTermsEnum(terms));
     assertEquals(0, automatonQueryNrHits(aq));
   }
+  
+  public void testHashCodeWithThreads() throws Exception {
+    final AutomatonQuery queries[] = new AutomatonQuery[1000];
+    for (int i = 0; i < queries.length; i++) {
+      queries[i] = new AutomatonQuery(new Term("bogus", "bogus"), AutomatonTestUtil.randomAutomaton(random()));
+    }
+    final CountDownLatch startingGun = new CountDownLatch(1);
+    int numThreads = TestUtil.nextInt(random(), 2, 5);
+    Thread[] threads = new Thread[numThreads];
+    for (int threadID = 0; threadID < numThreads; threadID++) {
+      Thread thread = new Thread() {
+          @Override
+          public void run() {
+            try {
+              startingGun.await();
+              for (int i = 0; i < queries.length; i++) {
+                queries[i].hashCode();
+              }
+            } catch (Exception e) {
+              Rethrow.rethrow(e);
+            }
+          }
+        };
+      threads[threadID] = thread;
+      thread.start();
+    }
+    startingGun.countDown();
+    for (Thread thread : threads) {
+      thread.join();
+    }
+  }
 }