summaryrefslogtreecommitdiffstats
path: root/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
diff options
context:
space:
mode:
authorShawn O. Pearce <sop@google.com>2010-11-15 14:03:28 -0800
committerShawn O. Pearce <sop@google.com>2010-11-15 15:32:46 -0800
commit617aa397e332857fd0009d66018c0434aa1517ad (patch)
treefd3a9a74ea70dc24fd3f126f513a39251511b4b6 /gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
parent936e709212e5e3a6b4f4bb17df38434918875182 (diff)
Work around buggy MyersDiff by killing threadsv2.1.6-rc1
The JGIt MyersDiff class contains a bug that triggers an infinite loop on only certain input files. Most source code is able to be processed in a reasonable time bound, but some just steal a thread and never return to the caller. Implement a custom thread pool that is used to invoke MyersDiff for the intraline difference data. If the worker thread doesn't end within the configured time bound (default of 5 seconds), Gerrit removes the worker from the pool and tries to kill the worker with the unsafe Thread.stop() method. A custom thread pool is used to try and make Thread.stop() safe by having the amount of data accessed by each worker thread be limited to only the "constant" inputs supplied by the cache lookup request, and the result that the thread would return. If any locks are released early as a result of ThreadDeath going up the worker thread stack at worst only the incoming or outgoing queues that are private to that worker will be corrupted. Since these queues are private to the worker, and to the thread that is currently borrowing this worker from the pool (and who is also now killing it), we can safely ensure that the queues won't be touched after the Thread.stop() request is made. This wouldn't be true if we reused any of the java.util.concurrent thread pool utilities. This change doesn't actually fix the MyersDiff bug, so we're leaving issue 487 open. It does however reduce the impact by trying to abort the runaway thread, and still show the file with intraline difference support disabled on just that one file. Bug: issue 487 Change-Id: I6cbfdd0acc6f7e612a29ed789efe9da591a45273 Signed-off-by: Shawn O. Pearce <sop@google.com>
Diffstat (limited to 'gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java')
-rw-r--r--gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java8
1 files changed, 1 insertions, 7 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
index c7bb08b4c8..a7cf10aa27 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCache.java
@@ -17,17 +17,11 @@ package com.google.gerrit.server.patch;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSet;
-import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.lib.ObjectId;
-
-import java.util.List;
-
/** Provides a cached list of {@link PatchListEntry}. */
public interface PatchListCache {
public PatchList get(PatchListKey key);
public PatchList get(Change change, PatchSet patchSet);
- public IntraLineDiff getIntraLineDiff(ObjectId aId, Text aText, ObjectId bId,
- Text bText, List<Edit> edits);
+ public IntraLineDiff getIntraLineDiff(IntraLineDiffKey key);
}