From 85b1b5c5e911cc8893893e418144ee86263fc178 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Fri, 26 Sep 2025 19:56:00 -0400 Subject: [PATCH] Fixed bug and race condition in the data type indexer --- .../core/datamgr/archive/DataTypeIndexer.java | 89 +++++++++++-------- .../datatype/CategoryPathSelectionEditor.java | 20 ++++- .../DataTypeDropDownSelectionDataModel.java | 4 +- 3 files changed, 70 insertions(+), 43 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/archive/DataTypeIndexer.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/archive/DataTypeIndexer.java index cc17249fce..057579e4a1 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/archive/DataTypeIndexer.java +++ b/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 dataTypeManagers = new ArrayList<>(); - private List 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 categoryPathList = Collections.emptyList(); + private volatile DataCache dataCache; + + private List 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 getSortedDataTypeList() { - - List newList = updateDataTypeList(); - - 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); + DataCache currentCache = updateDataCache(); + return currentCache.dataTypes(); } /** @@ -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 getSortedCategoryPathList() { - updateDataTypeList(); // the category list is quietly updated in the background - return categoryPathList; + public synchronized List getSortedCategoryPathList() { + DataCache currentCache = updateDataCache(); + return currentCache.categories(); } - private List 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 newDataTypeList = task.getList(); - categoryPathList = task.getCategoryPathList(); - return newDataTypeList; + // Store the new results, but check below to see if they were marked as stale while working. + // Either way, we will return these new results below. + 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 - // is possible that once marked stale, we may never have another request for this data - // again. - dataTypeList = Collections.emptyList(); - categoryPathList = Collections.emptyList(); + // Deleting this when stale allows us to free the memory. This is useful, since it is + // possible that once marked stale, we may never have another request for this data again. + dataCache = EMPTY_CACHE; } //================================================================================================== // Inner Classes //================================================================================================== + private record DataCache(List dataTypes, List 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 getList() { + List getDataTypes() { return dataTypes; } - List getCategoryPathList() { + List getCategoryPaths() { return categories; } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/CategoryPathSelectionEditor.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/CategoryPathSelectionEditor.java index 83d6f7a7b7..cb25651111 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/CategoryPathSelectionEditor.java +++ b/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 getMatchingDataRegex(Pattern p) { + private List getMatchingDataRegex(Pattern p, boolean startsWith) { List results = new ArrayList<>(); for (CategoryPath path : data) { - String pathString = path.getPath(); - Matcher m = p.matcher(pathString); + // use the name for 'startsWith' searches so users can avoid slashes or path data + String text = + startsWith ? CategoryPath.DELIMITER_CHAR + path.getName() : path.getPath(); + Matcher m = p.matcher(text); if (m.matches()) { results.add(path); } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/DataTypeDropDownSelectionDataModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/DataTypeDropDownSelectionDataModel.java index dd55039285..6fb9b36f92 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/datatype/DataTypeDropDownSelectionDataModel.java +++ b/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 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(); }