Merge remote-tracking branch 'origin/GP-6634_ghidragon_remove_refresh_method--SQUASHED'

Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBAnnotatedObject.java

@@ -281,17 +281,6 @@ public class DBAnnotatedObject extends DbObject {
 		return store.table.getRecord(key);
 	}
 
-	@Override
-	protected boolean refresh() {
-		try (LockHold hold = LockHold.lock(store.readLock())) {
-			return doRefresh(null);
-		}
-		catch (IOException e) {
-			adapter.dbError(e);
-			return false;
-		}
-	}
-
 	@Override
 	protected boolean refresh(DBRecord rec) {
 		try (LockHold hold = LockHold.lock(store.readLock())) {

Ghidra/Features/Base/src/test.slow/java/ghidra/program/database/ObjectCacheTest.java

@@ -194,10 +194,12 @@ public class ObjectCacheTest extends AbstractGhidraHeadedIntegrationTest {
 		}
 
 		@Override
-		protected boolean refresh() {
-			DBRecord refreshedRecord = database.get(record.getKey());
-			if (refreshedRecord != null) {
-				record = refreshedRecord;
+		protected boolean refresh(DBRecord rec) {
+			if (rec == null) {
+				rec = database.get(record.getKey());
+			}
+			if (rec != null) {
+				record = rec;
 				return true;
 			}
 			return false;

Ghidra/Features/FunctionID/src/main/java/ghidra/feature/fid/db/FunctionRecord.java

@@ -235,7 +235,7 @@ public class FunctionRecord extends DbObject implements FidHashQuad {
 	 * Never need to refresh...this database object is immutable.
 	 */
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		return false;
 	}
 }

Ghidra/Features/FunctionID/src/main/java/ghidra/feature/fid/db/StringRecord.java

@@ -15,6 +15,7 @@
  */
 package ghidra.feature.fid.db;
 
+import db.DBRecord;
 import ghidra.program.database.DbObject;
 
 /**
@@ -48,7 +49,7 @@ public class StringRecord extends DbObject {
 	 * Never need to refresh...this database object is immutable.
 	 */
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		return false;
 	}
 }

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/db/MarkupItemStorageDB.java

@@ -177,11 +177,6 @@ public class MarkupItemStorageDB extends DbObject implements MarkupItemStorage {
 		return this;
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord matchRecord) {
 		if (matchRecord == null) {

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/db/VTAssociationDB.java

@@ -78,11 +78,6 @@ public class VTAssociationDB extends DbObject implements VTAssociation {
 			getSourceAddress(), getDestinationAddress());
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord associationRecord) {
 		if (associationRecord == null) {

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/db/VTMatchDB.java

@@ -50,11 +50,6 @@ public class VTMatchDB extends DbObject implements VTMatch {
 		lock = session.getLock();
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord matchRecord) {
 		association = null;

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/db/VTMatchSetDB.java

@@ -289,7 +289,7 @@ public class VTMatchSetDB extends DbObject implements VTMatchSet {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		// MatchSets are not cached, so this method is not used
 		return true;
 	}

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/api/db/VTMatchTagDB.java

@@ -54,19 +54,20 @@ public class VTMatchTagDB extends DbObject implements VTMatchTag {
 	}
 
 	@Override
-	protected boolean refresh() {
-		DBRecord rec = null;
+	protected boolean refresh(DBRecord rec) {
 		try {
-			rec = sessionDB.getTagRecord(key);
+			if (rec == null) {
+				rec = sessionDB.getTagRecord(key);
+			}
+			if (rec != null) {
+				record = rec;
+				return true;
+			}
 		}
 		catch (IOException e) {
 			sessionDB.dbError(e);
 		}
-		if (rec == null) {
-			return false;
-		}
-		record = rec;
-		return true;
+		return false;
 	}
 
 	/**

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/DbObject.java

@@ -197,7 +197,7 @@ public abstract class DbObject {
 
 	/**
 	 * Internal method for performing a refresh on a database object. This method may be called
-	 * recursively, which is can detect and short circuit.
+	 * recursively, which it can detect and short circuit.
 	 * @param record a known valid record the object can use to refresh itself or null. If null
 	 * the object will have to do its own database retrieval of its record.
 	 */
@@ -258,20 +258,16 @@ public abstract class DbObject {
 	}
 
 	/**
-	 * Tells the object to refresh its state from the database.
-	 * 
-	 * @return true if the object was able to refresh itself. Return false if the object was
-	 *         deleted. Objects that extend this class must implement a refresh method. If an object
-	 *         can never refresh itself, then it should always return false.
-	 */
-	protected abstract boolean refresh();
-
-	/**
-	 * Tells the object to refresh its state from the database using the specified record if not
-	 * null. NOTE: The default implementation ignores the record and invokes refresh().
-	 * Implementations of this method must take care if multiple database tables are used since the
-	 * record supplied could correspond to another object. In some cases it may be best not to
-	 * override this method or ignore the record provided.
+	 * Tells the object to refresh its state from the database using the specified record if 
+	 * provided. The  record may be null and the object is generally expected to be able to 
+	 * retrieve its own record from the database as needed. Is is mostly passed as a parameter for
+	 * efficiency to keep an object from having to look up its record when the client already has
+	 * the record in hand such as when it is iterating over records.
+	 * <P>
+	 * This method generally should not be called directly as it provides no recursion protection. Instead,
+	 * most clients should call {@link #refreshIfNeeded()} instead which WILL provide recursion protection.
+	 * If clients must call this method, they either need to provided their own recursion protection or
+	 * be absolutely certain that recursive calls won't occur.
 	 * 
 	 * @param record valid record associated with object's key (optional, may be null to force
 	 *            record lookup or other refresh technique)
@@ -279,8 +275,6 @@ public abstract class DbObject {
 	 *         object was deleted. Objects that extend this class must implement a refresh method.
 	 *         If an object can never refresh itself, then it should always return false.
 	 */
-	protected boolean refresh(DBRecord record) {
-		return refresh();
-	}
+	protected abstract boolean refresh(DBRecord record);
 
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/bookmark/BookmarkDB.java

@@ -121,11 +121,6 @@ public class BookmarkDB extends DbObject implements Bookmark {
 		}
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord rec) {
 		if (rec == null) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java

@@ -80,11 +80,6 @@ abstract class CodeUnitDB extends DbObject implements CodeUnit, ProcessorContext
 		programContext = program.getProgramContext();
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord record) {
 		address = codeMgr.getAddressMap().decodeAddress(addr);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ArrayDB.java

@@ -73,13 +73,16 @@ class ArrayDB extends DataTypeDB implements Array {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
 			elementLength = -1;
-			DBRecord rec = adapter.getRecord(key);
+			if (rec == null) {
+				rec = adapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CategoryDB.java

@@ -89,11 +89,6 @@ class CategoryDB extends DbObject implements Category {
 		}
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord rec) {
 		subcategoryMap.clear();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java

@@ -289,13 +289,16 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = compositeAdapter.getRecord(key);
+			if (rec == null) {
+				rec = compositeAdapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
 				initialize();
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeDB.java

@@ -128,12 +128,10 @@ abstract class DataTypeDB extends DbObject implements DataType {
 		super.setDeleted();
 	}
 
-	@Override
-	protected boolean refresh() {
+	protected final void completeRefresh() {
 		category = null;
 		defaultSettings = null;
 		refreshName();
-		return true;
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumDB.java

@@ -524,14 +524,17 @@ class EnumDB extends DataTypeDB implements Enum {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
 			lazyEnumValues = null;
 			bitGroups = null;
-			DBRecord rec = adapter.getRecord(key);
+			if (rec == null) {
+				rec = adapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java

@@ -87,13 +87,16 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = funDefAdapter.getRecord(key);
+			if (rec == null) {
+				rec = funDefAdapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
 				loadParameters();
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/PointerDB.java

@@ -100,12 +100,15 @@ class PointerDB extends DataTypeDB implements Pointer {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = adapter.getRecord(key);
+			if (rec == null) {
+				rec = adapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/SourceArchiveDB.java

@@ -105,9 +105,11 @@ public class SourceArchiveDB extends DbObject implements SourceArchive {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = adapter.getRecord(key);
+			if (rec == null) {
+				rec = adapter.getRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
 				return true;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/TypedefDB.java

@@ -364,13 +364,16 @@ class TypedefDB extends DataTypeDB implements TypeDef {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = adapter.getRecord(key);
+			if (rec == null) {
+				rec = adapter.getRecord(key);
+			}
 			if (rec != null) {
 				settingsDef = null;
 				record = rec;
-				return super.refresh();
+				completeRefresh();
+				return true;
 			}
 		}
 		catch (IOException e) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionDB.java

@@ -815,7 +815,7 @@ public class FunctionDB extends DbObject implements Function {
 	 */
 	synchronized void endUpdate() {
 		if (--updateInProgressCount == 0 && updateRefreshRequired) {
-			refresh();
+			refresh(null);
 		}
 	}
 
@@ -923,11 +923,6 @@ public class FunctionDB extends DbObject implements Function {
 		}
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord refreshRec) {
 		if (updateInProgressCount != 0) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionTagDB.java

@@ -107,14 +107,6 @@ public class FunctionTagDB extends DbObject implements FunctionTag {
 		return record;
 	}
 
-	@Override
-	protected boolean refresh() {
-
-		// Call refresh with a null value to force the record
-		// to be refreshed using whatever is in the database.
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord rec) {
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/module/FragmentDB.java

@@ -59,9 +59,11 @@ class FragmentDB extends DbObject implements ProgramFragment {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = fragmentAdapter.getFragmentRecord(key);
+			if (rec == null) {
+				rec = fragmentAdapter.getFragmentRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
 				addrSet = moduleMgr.getFragmentAddressSet(key);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/module/ModuleDB.java

@@ -63,9 +63,11 @@ class ModuleDB extends DbObject implements ProgramModule {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		try {
-			DBRecord rec = moduleAdapter.getModuleRecord(key);
+			if (rec == null) {
+				rec = moduleAdapter.getModuleRecord(key);
+			}
 			if (rec != null) {
 				record = rec;
 				childCount = rec.getIntValue(ModuleDBAdapter.MODULE_CHILD_COUNT_COL);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/properties/PropertyMapDB.java

@@ -604,7 +604,7 @@ public abstract class PropertyMapDB<T> extends DbObject implements PropertyMap<T
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		cache = new ObjectCache<>(DEFAULT_CACHE_SIZE);
 		propertyTable = dbHandle.getTable(getTableName());
 		if (propertyTable != null) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/references/BigRefListV0.java

@@ -127,7 +127,7 @@ class BigRefListV0 extends RefList {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		return false;
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/references/RefListV0.java

@@ -119,11 +119,11 @@ class RefListV0 extends RefList {
 	}
 
 	@Override
-	protected boolean refresh() {
+	protected boolean refresh(DBRecord rec) {
 		return false;
 	}
 
-	// TODO: Try to elliminate - this is a little kludgey!
+	// TODO: Try to eliminate - this is a little kludgey!
 	byte[] getData() {
 		return refData;
 	}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/EquateDB.java

@@ -56,8 +56,10 @@ public class EquateDB extends DbObject implements Equate {
 	}
 
 	@Override
-	protected boolean refresh() {
-		DBRecord rec = equateMgr.getEquateRecord(key);
+	protected boolean refresh(DBRecord rec) {
+		if (rec == null) {
+			rec = equateMgr.getEquateRecord(key);
+		}
 		if (rec == null) {
 			return false;
 		}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/EquateRefDB.java

@@ -41,8 +41,10 @@ class EquateRefDB extends DbObject implements EquateReference {
 	}
 
 	@Override
-	protected boolean refresh() {
-		DBRecord rec = equateMgr.getEquateRefRecord(key);
+	protected boolean refresh(DBRecord rec) {
+		if (rec == null) {
+			rec = equateMgr.getEquateRefRecord(key);
+		}
 		if (rec == null) {
 			return false;
 		}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/SymbolDB.java

@@ -77,25 +77,24 @@ public abstract class SymbolDB extends DbObject implements Symbol {
 		return getName();
 	}
 
-	@Override
-	protected boolean refresh() {
-		return refresh(null);
-	}
-
 	@Override
 	protected boolean refresh(DBRecord rec) {
 		if (record != null) {
 			if (rec == null) {
 				rec = symbolMgr.getSymbolRecord(key);
 			}
-			if (rec == null || record.getByteValue(SymbolDatabaseAdapter.SYMBOL_TYPE_COL) != rec
-					.getByteValue(SymbolDatabaseAdapter.SYMBOL_TYPE_COL)) {
+			if (rec == null) {
+				return false; 
+			}
+			byte currentTypeValue = record.getByteValue(SymbolDatabaseAdapter.SYMBOL_TYPE_COL); 
+			byte newTypeValue = rec.getByteValue(SymbolDatabaseAdapter.SYMBOL_TYPE_COL);
+			if (newTypeValue != currentTypeValue) {
 				return false;
 			}
 			record = rec;
-			address = symbolMgr.getAddressMap()
+			Address newAddress = symbolMgr.getAddressMap()
 					.decodeAddress(rec.getLongValue(SymbolDatabaseAdapter.SYMBOL_ADDR_COL));
-			return true;
+			return address.equals(newAddress);
 		}
 		return false;
 	}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/VariableStorageManagerDB.java

@@ -210,24 +210,24 @@ public class VariableStorageManagerDB implements VariableStorageManager {
 		}
 
 		@Override
-		protected boolean refresh() {
+		protected boolean refresh(DBRecord rec) {
 			try {
 				storage = VariableStorage.BAD_STORAGE;
-				if (record != null) {
-					DBRecord rec = adapter.getRecord(key);
-					if (rec == null) {
-						return false;
-					}
-					record = rec;
-					try {
-						storage = VariableStorage.deserialize(arch,
-							record.getString(VariableStorageDBAdapter.STORAGE_COL));
-					}
-					catch (InvalidInputException e) {
-						// treat as bad storage
-					}
-					return true;
+				if (rec == null) {
+					rec = adapter.getRecord(key);
 				}
+				if (rec == null) {
+					return false;
+				}
+				record = rec;
+				try {
+					storage = VariableStorage.deserialize(arch,
+						record.getString(VariableStorageDBAdapter.STORAGE_COL));
+				}
+				catch (InvalidInputException e) {
+					// treat as bad storage
+				}
+				return true;
 			}
 			catch (IOException e) {
 				errorHandler.dbError(e);