Bug 379123 - [Markers] AIOOBE in MarkerCategory.getChildren

MarkerCategory.children is computed from a volatile
Markers.markerEntryArray, which may change *after* the MarkerCategory is
created. Also MarkerCategory.children can be set to null at any time by
Markers.sortMarkerEntries(), but MarkerCategory.getChildren() should
never return null.

 - Check now the if markerEntryArray size matches to our indices and
return empty children array in case we are out of date.
 - Made children volatile and avoiding using it directly to avoid MT
issues.

Change-Id: Ie5c8dd8c13abd5000a40428b01f81379eb2dfde6
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

2 files changed, 30 insertions, 18 deletions

diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerCategory.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerCategory.java
index 2b0894408f6..bcd58ce7c14 100644
--- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerCategory.java
+++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerCategory.java
@@ -18,19 +18,17 @@ import org.eclipse.ui.views.markers.internal.MarkerMessages;
 
 class MarkerCategory extends MarkerSupportItem {
 
-	boolean refreshing;
+	final int start;
 
-	int start;
+	final int end;
 
-	int end;
+	private volatile MarkerEntry[] children;
 
-	MarkerEntry[] children;
-
-	private String name;
+	private final String name;
 
 	private int severity = -1;
 
-	private Markers markers;
+	private final Markers markers;
 
 	/**
 	 * Create a new instance of the receiver that has the markers between
@@ -46,22 +44,36 @@ class MarkerCategory extends MarkerSupportItem {
 		this.markers = markers;
 		start = startIndex;
 		end = endIndex;
-		refreshing=false;
 		name = categoryName;
 	}
 
 	@Override
 	MarkerSupportItem[] getChildren() {
-		if (children == null) {
-			MarkerItem[] allMarkers = markers.getMarkerEntryArray();
-			int totalSize = getChildrenCount();
-			children = new MarkerEntry[totalSize];
-			System.arraycopy(allMarkers, start, children, 0, totalSize);
-			for (MarkerEntry markerEntry : children) {
-				markerEntry.setCategory(this);
-			}
+		MarkerEntry[] myChildren = children;
+		if (myChildren != null) {
+			return myChildren;
+		}
+		MarkerItem[] allMarkers = markers.getMarkerEntryArray();
+		int markersLength = allMarkers.length;
+		if (start >= markersLength || end >= markersLength) {
+			// NB: the array can be changed after our creation via
+			// markers::updateWithNewMarkers so that the expected array size doesn't match
+			// anymore to our start/end values. Just return nothing in this case and let the
+			// "children" be null to avoid persistence of inconsistent data
+			return new MarkerEntry[0];
 		}
-		return children;
+		int totalSize = getChildrenCount();
+		myChildren = new MarkerEntry[totalSize];
+		System.arraycopy(allMarkers, start, myChildren, 0, totalSize);
+		for (MarkerEntry markerEntry : myChildren) {
+			markerEntry.setCategory(this);
+		}
+		children = myChildren;
+		return myChildren;
+	}
+
+	void resetChildren() {
+		children = null;
 	}
 
 	@Override
diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java
index 75ddb89d2ab..9d621e806e5 100644
--- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java
+++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java
@@ -158,7 +158,7 @@ class Markers {
 					}
 					lastCategory = category;
 					// sort various categories
-					category.children = null; // reset cached children
+					category.resetChildren(); // reset cached children
 					int avaliable = category.end - category.start + 1;
 					int effLimit = getShowingLimit(avaliable);
 					MarkerSortUtil.sortStartingKElement(markerEntryArray,