Merge remote-tracking branch 'origin/GP-1-dragonmacher-category-chooser-fix'

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/archive/DataTypeIndexer.java

@@ -17,9 +17,8 @@ package ghidra.app.plugin.core.datamgr.archive;
 
 import java.util.*;
 
-import javax.swing.SwingUtilities;
-
 import ghidra.program.model.data.*;
+import ghidra.util.Swing;
 import ghidra.util.task.*;
 
 /**
@@ -30,12 +29,14 @@ import ghidra.util.task.*;
  * changed.
  */
 public class DataTypeIndexer {
-	private List<DataTypeManager> dataTypeManagers = new ArrayList<>();
-	private List<DataType> dataTypeList = Collections.emptyList();
-	private DataTypeIndexUpdateListener listener = new DataTypeIndexUpdateListener();
 
+	private static final DataCache EMPTY_CACHE =
+		new DataCache(Collections.emptyList(), Collections.emptyList());
 	private volatile boolean isStale = true;
-	private List<CategoryPath> categoryPathList = Collections.emptyList();
+	private volatile DataCache dataCache;
+
+	private List<DataTypeManager> dataTypeManagers = new ArrayList<>();
+	private DataTypeIndexUpdateListener listener = new DataTypeIndexUpdateListener();
 
 	// Note: synchronizing here prevents concurrent mod issues with the managers list
 	public synchronized void addDataTypeManager(DataTypeManager dataTypeManager) {
@@ -65,20 +66,8 @@ public class DataTypeIndexer {
 	 * @return a sorted list of the data types open in the current tool.
 	 */
 	public synchronized List<DataType> getSortedDataTypeList() {
-
+		DataCache currentCache = updateDataCache();
-		List<DataType> newList = updateDataTypeList();
+		return currentCache.dataTypes();
-
-		if (isStale) {
-			//
-			// 					Unusual Code Alert!
-			// Don't save the list we just made, as it is already stale again due to changes
-			// to the Data Type Managers that happened while we were building.
-			//
-			return newList;
-		}
-
-		dataTypeList = newList;
-		return Collections.unmodifiableList(newList);
 	}
 
 	/**
@@ -88,47 +77,71 @@ public class DataTypeIndexer {
 	 * @return a list of the {@link CategoryPath} associated with the data types open in the 
 	 * current tool.
 	 */
-	public List<CategoryPath> getSortedCategoryPathList() {
+	public synchronized List<CategoryPath> getSortedCategoryPathList() {
-		updateDataTypeList(); // the category list is quietly updated in the background 
+		DataCache currentCache = updateDataCache();
-		return categoryPathList;
+		return currentCache.categories();
 	}
 
-	private List<DataType> updateDataTypeList() {
+	private synchronized DataCache updateDataCache() {
+
+		/*
+		 	This method is super complicated due to the fact that our cache can be invalidated in a
+		 	non-synchronized call.  This method is synchronized to prevent multiple external clients
+		 	from triggering a data load when the cache is stale.  However, system events may 
+		 	invalidate the cache while we are in the process of getting new data.  We let those 
+		 	events clear the cache outside of a synchronized block to avoid blocking the swing 
+		 	thread.  
+		 */
+		DataCache currentCache = dataCache;
 		if (!isStale) {
-			return dataTypeList;
+			return currentCache; // the current cache is not stale; no need to rebuild
 		}
 
-		// set the flag here to handle the case where changes are made while we are building
+		// mark as not stale (this can be changed at any time from external events)
 		isStale = false;
 
 		IndexerTask task = new IndexerTask();
-		if (SwingUtilities.isEventDispatchThread()) {
+		if (Swing.isSwingThread()) {
 			TaskLauncher.launch(task);
 		}
 		else {
 			task.run(TaskMonitor.DUMMY);
 		}
 
-		List<DataType> newDataTypeList = task.getList();
+		// Store the new results, but check below to see if they were marked as stale while working.
-		categoryPathList = task.getCategoryPathList();
+		// Either way, we will return these new results below.
-		return newDataTypeList;
+		DataCache newCache = new DataCache(task.getDataTypes(), task.getCategoryPaths());
+		dataCache = newCache;
+
+		if (isStale) {
+			// There is a chance, while we were working, the cache was marked as stale.  Assign the
+			// new cache and then clear it here if we hit that case.  The results we return below
+			// will be valid for the client for this call only.
+			// Note: we have guilty knowledge of the markStale() method, which sets the flag first
+			//       and then clears the cache.
+			dataCache = EMPTY_CACHE;
+		}
+
+		return newCache;
 	}
 
-	// Note: purposefully not synchronized for speed
+	// Note: intentionally not synchronized for speed; called from system events
 	private void markStale() {
 		isStale = true;
 
-		// Deleting this when stale allows us to free the memory.  This is useful, since it
+		// Deleting this when stale allows us to free the memory.  This is useful, since it is 
-		// is possible that once marked stale, we may never have another request for this data
+		// possible that once marked stale, we may never have another request for this data again.
-		// again.
+		dataCache = EMPTY_CACHE;
-		dataTypeList = Collections.emptyList();
-		categoryPathList = Collections.emptyList();
 	}
 
 //==================================================================================================
 // Inner Classes
 //==================================================================================================
 
+	private record DataCache(List<DataType> dataTypes, List<CategoryPath> categories) {
+
+	}
+
 	// We use a case-insensitive sort on the data since clients may perform case-insensitive 
 	// searches.  This class was using the DataTypeComparator.INSTANCE for sorting, which is 
 	// case-sensitive.  This produced cases where not all matching data were found during queries, 
@@ -239,11 +252,11 @@ public class DataTypeIndexer {
 			}
 		}
 
-		List<DataType> getList() {
+		List<DataType> getDataTypes() {
 			return dataTypes;
 		}
 
-		List<CategoryPath> getCategoryPathList() {
+		List<CategoryPath> getCategoryPaths() {
 			return categories;
 		}
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/CategoryPathSelectionEditor.java

@@ -432,14 +432,26 @@ public class CategoryPathSelectionEditor extends AbstractCellEditor {
 			}
 
 			Pattern p = mode.createPattern(searchText);
-			return getMatchingDataRegex(p);
+			boolean startsWith = mode == SearchMode.STARTS_WITH;
+
+			if (startsWith) {
+				// update the 'starts with' pattern to allow for optional leading slash, as that is
+				// sometimes intuitive for the user to type.
+				String pattern = p.pattern();
+				String newPattern = "/{0,1}" + pattern;
+				p = Pattern.compile(newPattern, Pattern.CASE_INSENSITIVE);
+			}
+
+			return getMatchingDataRegex(p, startsWith);
 		}
 
-		private List<CategoryPath> getMatchingDataRegex(Pattern p) {
+		private List<CategoryPath> getMatchingDataRegex(Pattern p, boolean startsWith) {
 			List<CategoryPath> results = new ArrayList<>();
 			for (CategoryPath path : data) {
-				String pathString = path.getPath();
+				// use the name for 'startsWith' searches so users can avoid slashes or path data
-				Matcher m = p.matcher(pathString);
+				String text =
+					startsWith ? CategoryPath.DELIMITER_CHAR + path.getName() : path.getPath();
+				Matcher m = p.matcher(text);
 				if (m.matches()) {
 					results.add(path);
 				}

Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/DataTypeDropDownSelectionDataModel.java

@@ -23,6 +23,8 @@ import java.util.regex.Pattern;
 import javax.help.UnsupportedOperationException;
 import javax.swing.*;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.widgets.DropDownSelectionTextField;
 import docking.widgets.DropDownTextFieldDataModel;
 import docking.widgets.list.GListCellRenderer;
@@ -100,7 +102,7 @@ public class DataTypeDropDownSelectionDataModel implements DropDownTextFieldData
 
 	@Override
 	public List<DataType> getMatchingData(String searchText, SearchMode mode) {
-		if (searchText == null || searchText.length() == 0) {
+		if (StringUtils.isBlank(searchText)) {
 			// full list results not supported since the data may be too large for user interaction
 			return Collections.emptyList();
 		}