bug 191522: provide full text search functionality over task comments
https://bugs.eclipse.org/bugs/show_bug.cgi?id=191522
improvements to thread safety and concurrency model
diff --git a/org.eclipse.mylyn.tasks.index.core/src/org/eclipse/mylyn/internal/tasks/index/core/TaskListIndex.java b/org.eclipse.mylyn.tasks.index.core/src/org/eclipse/mylyn/internal/tasks/index/core/TaskListIndex.java
index 2fae2b5..5e2cd48 100644
--- a/org.eclipse.mylyn.tasks.index.core/src/org/eclipse/mylyn/internal/tasks/index/core/TaskListIndex.java
+++ b/org.eclipse.mylyn.tasks.index.core/src/org/eclipse/mylyn/internal/tasks/index/core/TaskListIndex.java
@@ -25,6 +25,9 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.Logger;
import org.apache.lucene.document.DateTools;
@@ -198,6 +201,8 @@
private int maxMatchSearchHits = 1500;
+ private final ReadWriteLock indexReaderLock = new ReentrantReadWriteLock(true);
+
private TaskListIndex(TaskList taskList, TaskDataManager dataManager) {
if (taskList == null) {
throw new IllegalArgumentException();
@@ -323,11 +328,14 @@
if (patternString.equals(COMMAND_RESET_INDEX)) {
reindex();
}
- IndexReader indexReader = getIndexReader();
- if (indexReader != null) {
- Set<String> hits;
+ Lock readLock = indexReaderLock.readLock();
+ readLock.lock();
+ try {
- synchronized (indexReader) {
+ IndexReader indexReader = getIndexReader();
+ if (indexReader != null) {
+ Set<String> hits;
+
if (lastResults == null || (lastPatternString == null || !lastPatternString.equals(patternString))) {
this.lastPatternString = patternString;
@@ -359,15 +367,18 @@
} else {
hits = lastResults;
}
- }
- synchronized (this) {
- if (this.indexReader == indexReader) {
- this.lastPatternString = patternString;
- this.lastResults = hits;
+ synchronized (this) {
+ if (this.indexReader == indexReader) {
+ this.lastPatternString = patternString;
+ this.lastResults = hits;
+ }
}
+ String taskIdentifier = task.getHandleIdentifier();
+ return hits != null && hits.contains(taskIdentifier);
}
- String taskIdentifier = task.getHandleIdentifier();
- return hits != null && hits.contains(taskIdentifier);
+
+ } finally {
+ readLock.unlock();
}
return false;
}
@@ -392,30 +403,37 @@
}
public void find(String patternString, TaskCollector collector, int resultsLimit) {
- IndexReader indexReader = getIndexReader();
- if (indexReader != null) {
- IndexSearcher indexSearcher = new IndexSearcher(indexReader);
- try {
- Query query = computeQuery(patternString);
- TopDocs results = indexSearcher.search(query, resultsLimit);
- for (ScoreDoc scoreDoc : results.scoreDocs) {
- Document document = indexReader.document(scoreDoc.doc);
- String taskIdentifier = document.get(IndexField.IDENTIFIER.fieldName());
- AbstractTask task = taskList.getTask(taskIdentifier);
- if (task != null) {
- collector.collect(task);
+
+ Lock readLock = indexReaderLock.readLock();
+ readLock.lock();
+ try {
+ IndexReader indexReader = getIndexReader();
+ if (indexReader != null) {
+ IndexSearcher indexSearcher = new IndexSearcher(indexReader);
+ try {
+ Query query = computeQuery(patternString);
+ TopDocs results = indexSearcher.search(query, resultsLimit);
+ for (ScoreDoc scoreDoc : results.scoreDocs) {
+ Document document = indexReader.document(scoreDoc.doc);
+ String taskIdentifier = document.get(IndexField.IDENTIFIER.fieldName());
+ AbstractTask task = taskList.getTask(taskIdentifier);
+ if (task != null) {
+ collector.collect(task);
+ }
+ }
+ } catch (IOException e) {
+ StatusHandler.fail(new Status(IStatus.ERROR, TasksIndexCore.BUNDLE_ID,
+ "Unexpected failure within task list index", e)); //$NON-NLS-1$
+ } finally {
+ try {
+ indexSearcher.close();
+ } catch (IOException e) {
+ e.printStackTrace();
}
}
- } catch (IOException e) {
- StatusHandler.fail(new Status(IStatus.ERROR, TasksIndexCore.BUNDLE_ID,
- "Unexpected failure within task list index", e)); //$NON-NLS-1$
- } finally {
- try {
- indexSearcher.close();
- } catch (IOException e) {
- e.printStackTrace();
- }
}
+ } finally {
+ readLock.unlock();
}
}
@@ -468,22 +486,26 @@
// ignore
}
- synchronized (this) {
- if (indexReader != null) {
- synchronized (indexReader) {
+ Lock writeLock = indexReaderLock.writeLock();
+ writeLock.lock();
+ try {
+ synchronized (this) {
+ if (indexReader != null) {
try {
indexReader.close();
} catch (IOException e) {
e.printStackTrace();
}
+ indexReader = null;
}
- indexReader = null;
}
- }
- try {
- directory.close();
- } catch (IOException e) {
- e.printStackTrace();
+ try {
+ directory.close();
+ } catch (IOException e) {
+ e.printStackTrace();
+ }
+ } finally {
+ writeLock.unlock();
}
}
@@ -802,13 +824,17 @@
}
monitor.worked(WORK_PER_SEGMENT);
}
- synchronized (TaskListIndex.this) {
- if (indexReader != null) {
- synchronized (indexReader) {
+ Lock writeLock = indexReaderLock.writeLock();
+ writeLock.lock();
+ try {
+ synchronized (TaskListIndex.this) {
+ if (indexReader != null) {
indexReader.close();
+ indexReader = null;
}
- indexReader = null;
}
+ } finally {
+ writeLock.unlock();
}
} catch (CoreException e) {
throw e;
diff --git a/org.eclipse.mylyn.tasks.index.tests/src/org/eclipse/mylyn/internal/tasks/index/tests/TaskListIndexTest.java b/org.eclipse.mylyn.tasks.index.tests/src/org/eclipse/mylyn/internal/tasks/index/tests/TaskListIndexTest.java
index ee82c2f..6ae7592 100644
--- a/org.eclipse.mylyn.tasks.index.tests/src/org/eclipse/mylyn/internal/tasks/index/tests/TaskListIndexTest.java
+++ b/org.eclipse.mylyn.tasks.index.tests/src/org/eclipse/mylyn/internal/tasks/index/tests/TaskListIndexTest.java
@@ -11,7 +11,7 @@
package org.eclipse.mylyn.internal.tasks.index.tests;
-import static org.junit.Assert.assertEquals;
+import static junit.framework.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@@ -20,12 +20,20 @@
import java.io.IOException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
import java.util.logging.Logger;
+import junit.framework.Assert;
+
import org.eclipse.core.runtime.CoreException;
import org.eclipse.mylyn.commons.core.DelegatingProgressMonitor;
import org.eclipse.mylyn.internal.tasks.core.AbstractTask;
@@ -318,4 +326,56 @@
// should now match
assertTrue(index.matches(repositoryTask, content));
}
+
+ @Test
+ public void testMultithreadedAccessOnFind() throws CoreException, InterruptedException, ExecutionException {
+ setupIndex();
+
+ final ITask repositoryTask = context.createRepositoryTask();
+
+ index.waitUntilIdle();
+ index.setDefaultField(IndexField.CONTENT);
+
+ final int nThreads = 10;
+ final int[] concurrencyLevel = new int[1];
+ ExecutorService executorService = Executors.newFixedThreadPool(nThreads);
+ try {
+ Collection<Callable<Object>> tasks = new HashSet<Callable<Object>>();
+ for (int x = 0; x < nThreads; ++x) {
+ tasks.add(new Callable<Object>() {
+
+ public Object call() throws Exception {
+ final int[] hitCount = new int[1];
+ index.find(repositoryTask.getSummary(), new TaskCollector() {
+
+ @Override
+ public void collect(ITask task) {
+ synchronized (concurrencyLevel) {
+ ++concurrencyLevel[0];
+ if (concurrencyLevel[0] < nThreads) {
+ try {
+ concurrencyLevel.wait(5000L);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ } else {
+ concurrencyLevel.notifyAll();
+ }
+ }
+ ++hitCount[0];
+ }
+ }, 100);
+ return hitCount[0] == 1;
+ }
+ });
+ }
+ List<Future<Object>> futures = executorService.invokeAll(tasks);
+ for (Future<Object> future : futures) {
+ assertEquals(Boolean.TRUE, future.get());
+ }
+ Assert.assertEquals(nThreads, concurrencyLevel[0]);
+ } finally {
+ executorService.shutdownNow();
+ }
+ }
}
