Merge branch 'GP-4692_ghidra1_CorrectedInUseCheckinFailure' into patch

NationalSecurityAgency · Jun 14, 2024 · 4353380 · 4353380
2 parents 5baaa82 + bac18fe
commit 4353380
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 20 deletions.
diff --git a/Ghidra/Framework/DB/src/main/java/db/buffers/BufferMgr.java b/Ghidra/Framework/DB/src/main/java/db/buffers/BufferMgr.java
@@ -78,8 +78,8 @@ public class BufferMgr {
 	 */
 	private BufferNode cacheHead;
 	private BufferNode cacheTail;
-	private int cacheSize = 0;
-	private int buffersOnHand = 0;
+	private int cacheSize;
+	private int buffersOnHand;
 	private int lockCount = 0;
 
 	/**
@@ -235,6 +235,10 @@ private BufferMgr(BufferFile sourceFile, int requestedBufferSize, long approxCac
 
 	private void initializeCache() throws IOException {
 
+		if (lockCount != 0) {
+			throw new IOException("Unable to re-initialize buffer cache while in-use");
+		}
+
 		if (cacheFile != null) {
 			cacheFile.delete();
 		}
@@ -244,6 +248,9 @@ private void initializeCache() throws IOException {
 		cacheTail = new BufferNode(TAIL, -1);
 		cacheHead.nextCached = cacheTail;
 		cacheTail.prevCached = cacheHead;
+
+		cacheSize = 0;
+		buffersOnHand = 0;
 
 		// Create disk cache file
 		cacheFile = new LocalBufferFile(bufferSize, CACHE_FILE_PREFIX, CACHE_FILE_EXT);
@@ -257,6 +264,8 @@ private void initializeCache() throws IOException {
 				cacheFile.setParameter(name, sourceFile.getParameter(name));
 			}
 		}
+
+		resetCacheStatistics();
 
 		if (alwaysPreCache) {
 			startPreCacheIfNeeded();
@@ -2058,7 +2067,7 @@ public int getLowBufferCount() {
 	public void resetCacheStatistics() {
 		cacheHits = 0;
 		cacheMisses = 0;
-		lowWaterMark = cacheSize;
+		lowWaterMark = cacheSize - 1;
 	}
 
 	public String getStatusInfo() {

diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFileData.java b/Ghidra/Framework/Project/src/main/java/ghidra/framework/data/GhidraFileData.java
@@ -1149,7 +1149,7 @@ else if (folderItem instanceof DataFileItem) {
 
 				if (keepCheckedOut) {
 
-					// Maintain exclusive chekout if private repository or file is open for update
+					// Maintain exclusive checkout if private repository or file is open for update
 					boolean exclusive = !versionedFileSystem.isShared() || (inUseDomainObj != null);
 
 					ProjectLocator projectLocator = parent.getProjectLocator();
@@ -1169,6 +1169,11 @@ else if (folderItem instanceof DataFileItem) {
 							projectLocator.isTransient()));
 					folderItem.setCheckout(checkout.getCheckoutId(), exclusive,
 						checkout.getCheckoutVersion(), folderItem.getCurrentVersion());
+
+					if (inUseDomainObj != null) {
+						// Reset source file and change-sets for open database
+						getContentHandler().resetDBSourceFile(folderItem, inUseDomainObj);
+					}
 				}
 				else {
 					// NOTE: file open read-only may prevent removal and result in hijack
@@ -1180,10 +1185,7 @@ else if (folderItem instanceof DataFileItem) {
 						// Ignore - should result in Hijacked file
 					}
 				}
-
-				if (inUseDomainObj != null) {
-					getContentHandler().resetDBSourceFile(folderItem, inUseDomainObj);
-				}
+
 			} // end of synchronized block
 
 			if (inUseDomainObj != null) {
@@ -1535,15 +1537,16 @@ void checkin(CheckinHandler checkinHandler, TaskMonitor monitor)
 							}
 						}
 					}
+
+					if (inUseDomainObj != null) {
+						// Reset source file and change-sets for open database
+						contentHandler.resetDBSourceFile(folderItem, inUseDomainObj);
+					}
 				}
 				else {
 					undoCheckout(false, true);
 				}
 
-				if (inUseDomainObj != null) {
-					contentHandler.resetDBSourceFile(folderItem, inUseDomainObj);
-				}
-
 			} // end of synchronized block
 
 			if (inUseDomainObj != null) {
@@ -1915,6 +1918,8 @@ void merge(boolean okToUpgrade, TaskMonitor monitor)
 		try {
 			inUseDomainObj = getAndLockInUseDomainObjectForMergeUpdate("merge");
 
+			ContentHandler<?> contentHandler = getContentHandler();
+
 			if (!modifiedSinceCheckout()) {
 				// Quick merge
 				folderItem.updateCheckout(versionedFolderItem, true, monitor);
@@ -1925,8 +1930,6 @@ void merge(boolean okToUpgrade, TaskMonitor monitor)
 					throw new IOException("Merge failed, merge is not supported in headless mode");
 				}
 
-				ContentHandler<?> contentHandler = getContentHandler();
-
 				// Test versioned file for VersionException
 				int mergeVer = versionedFolderItem.getCurrentVersion();
 				if (!okToUpgrade) {
@@ -1995,14 +1998,13 @@ void merge(boolean okToUpgrade, TaskMonitor monitor)
 				versionedFolderItem.updateCheckoutVersion(checkoutId, mergeVer,
 					ClientUtil.getUserName());
 				tmpItem = null;
-				Msg.info(this, "Merge completed for " + name);
-
-				if (inUseDomainObj != null) {
-					contentHandler.resetDBSourceFile(folderItem, inUseDomainObj);
-				}
 			}
+
+			Msg.info(this, "Updated checkout completed for " + name);
 
 			if (inUseDomainObj != null) {
+				// Reset source file and change-sets for open database
+				contentHandler.resetDBSourceFile(folderItem, inUseDomainObj);
 				inUseDomainObj.invalidate();
 			}
 		}

diff --git a/...tionTest/src/test.slow/java/ghidra/framework/main/datatree/VersionControlAction2Test.java b/...tionTest/src/test.slow/java/ghidra/framework/main/datatree/VersionControlAction2Test.java
@@ -36,6 +36,8 @@
 import ghidra.framework.main.projectdata.actions.VersionControlAction;
 import ghidra.framework.model.DomainFile;
 import ghidra.framework.model.DomainFolder;
+import ghidra.program.database.ProgramDB;
+import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.listing.CodeUnit;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.symbol.SourceType;
@@ -219,7 +221,7 @@ public void testCheckIn() throws Exception {
 		waitForSwing();
 		waitForTasks();
 
-		Program program = (Program) ((DomainFileNode) node).getDomainFile()
+		ProgramDB program = (ProgramDB) ((DomainFileNode) node).getDomainFile()
 				.getDomainObject(this,
 					true, false, TaskMonitor.DUMMY);
 		int transactionID = program.startTransaction("test");
@@ -253,6 +255,58 @@ public void testCheckIn() throws Exception {
 		assertTrue(!df.isCheckedOut());
 
 	}
+
+	@Test
+	public void testCheckInWhileOpen() throws Exception {
+		GTreeNode node = getNode(PROGRAM_A);
+		addToVersionControl(node, false);
+
+		selectNode(node);
+		DockingActionIf action = getAction("CheckOut");
+		runSwing(() -> action.actionPerformed(getDomainFileActionContext(node)), false);
+		waitForSwing();
+		waitForTasks();
+
+		ProgramDB program = (ProgramDB) ((DomainFileNode) node).getDomainFile()
+				.getDomainObject(this,
+					true, false, TaskMonitor.DUMMY);
+		int transactionID = program.startTransaction("test");
+		try {
+			// Ensure that buffer memory cache has been completely consumed
+			// Max BufferMgr cache size is 256*16KByte=4MByte
+			AddressSpace space = program.getAddressFactory().getDefaultAddressSpace();
+			program.getMemory().createInitializedBlock("BigBlock", space.getAddress(0x80000000L), 
+				4*1024*1024, (byte)0xff, TaskMonitor.DUMMY, false);
+		}
+		finally {
+			program.endTransaction(transactionID, true);
+			program.save(null, TaskMonitor.DUMMY);
+		}
+
+		try {
+			DockingActionIf checkInAction = getAction("CheckIn");
+			runSwing(() -> checkInAction.actionPerformed(getDomainFileActionContext(node)), false);
+			waitForSwing();
+			VersionControlDialog dialog = waitForDialogComponent(VersionControlDialog.class);
+			assertNotNull(dialog);
+			JTextArea textArea = findComponent(dialog, JTextArea.class);
+			assertNotNull(textArea);
+			JCheckBox cb = findComponent(dialog, JCheckBox.class);
+			assertNotNull(cb);
+			runSwing(() -> {
+				textArea.setText("This is a test");
+				cb.setSelected(false);
+			});
+			pressButtonByText(dialog, "OK");
+			waitForTasks();
+			DomainFile df = ((DomainFileNode) node).getDomainFile();
+			assertTrue(df.isCheckedOut());
+		}
+		finally {
+			program.release(this);
+		}
+
+	}
 
 	@Test
 	public void testDeleteVersionCheckedOut() throws Exception {