GP-2181 - removed JMockit usage from some tests (part 3)

Ghidra/Features/Base/src/test.slow/java/ghidra/framework/data/TransactionLockingTest.java

@@ -15,27 +15,25 @@
  */
 package ghidra.framework.data;
 
-import static org.junit.Assert.assertFalse;
-
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
 
-import org.junit.*;
+import org.junit.Before;
+import org.junit.Test;
 
 import generic.test.AbstractGenericTest;
 import ghidra.framework.model.DomainObjectLockedException;
 import ghidra.program.database.ProgramBuilder;
 import ghidra.program.model.listing.Program;
 import ghidra.util.exception.AssertException;
-import mockit.*;
 
 public class TransactionLockingTest extends AbstractGenericTest {
 
 	private Program program;
 
-	private CountDownLatch programLockedLatch = new CountDownLatch(1);
-	private CountDownLatch lockExceptionLatch = new CountDownLatch(1);
+	// placeholder for exceptions encountered while testing; this will be checked at the end
+	private Exception unexpectedException = null;
 
 	@Before
 	public void setUp() throws Exception {
@@ -43,80 +41,89 @@ public class TransactionLockingTest extends AbstractGenericTest {
 		program = builder.getProgram();
 	}
 
-	@After
-	public void tearDown() {
-		program.release(this);
-	}
-
+	/**
+	 * This test attempts to verify 2 things: 1) that a client attempting to start a transaction
+	 * on a locked domain object will get an exception, and 2) the client will keep waiting after
+	 * getting the exception, due to the use of a while(true) loop in the system under test.
+	 */
 	@Test
-	public void testTransactionWaitForLock() throws Exception {
+	public void testTransactionWaitForLock() {
 
-		// Inject DomainObjectLockedException mock to trigger lockExceptionLatch
-		new SpyDomainObjectLockedException();
+		//
+		// The latch uses a count of 3, which is an arbitrary number greater than 1.  This allows
+		// us to know that the system performed the expected spin/wait loop more than once.
+		//
+		CountDownLatch clientWaitLatch = new CountDownLatch(3);
+		Function<DomainObjectLockedException, Boolean> exceptionHandler = e -> {
+			clientWaitLatch.countDown();
+			return true; // keep waiting; the client will get released by the lock thread
+		};
 
-		AtomicReference<Exception> exceptionRef = new AtomicReference<>();
-
-		// setup transaction thread
+		//
+		// Thread that will block while waiting to start the transaction
+		//
+		CountDownLatch lockThreadLatch = new CountDownLatch(1);
 		Thread txThread = new Thread(() -> {
 			try {
-				programLockedLatch.await(2, TimeUnit.SECONDS);
-				int txId = program.startTransaction("Test"); // Block until lock released
+				lockThreadLatch.await(2, TimeUnit.SECONDS);
+
+				//
+				// Cast to db to use package-level method for testing.
+				//
+				// The call to startTransaction() will block until our exception handler signals
+				// to continue.
+				//
+				DomainObjectAdapterDB programDb = (DomainObjectAdapterDB) program;
+				int txId = programDb.startTransaction("Test", null, exceptionHandler);
 				program.endTransaction(txId, true);
 			}
 			catch (Exception e) {
-				exceptionRef.set(e);
+				unexpectedException = e;
 			}
 
-		}, "Tx-Thread");
+		}, "Tx-Client-Thread");
 		txThread.start();
 
-		//setup lock thread
+		//
+		// Thread that will acquire the program lock, which will block the other thread
+		//
 		Thread lockThread = new Thread(() -> {
-			boolean gotLock = program.lock("TestLock");
-			if (!gotLock) {
-				exceptionRef.set(new AssertException("Failed to obtain lock"));
+			if (!program.lock("TestLock")) {
+				unexpectedException = new AssertException("Failed to obtain program lock");
+				lockThreadLatch.countDown(); // signal for the Transaction Thread to proceed
+				return;
 			}
 
-			programLockedLatch.countDown(); // signal for startTransaction
-
-			if (gotLock) {
+			lockThreadLatch.countDown(); // signal for the Transaction Thread to proceed
 			try {
-					lockExceptionLatch.await(2, TimeUnit.SECONDS);
+				// keep blocking until we know the client has waited for our test exception handler
+				// to signal to continue
+				clientWaitLatch.await(2, TimeUnit.SECONDS);
 			}
 			catch (InterruptedException e) {
-					// unexpected
-					exceptionRef.set(e);
+				unexpectedException = new AssertException(
+					"Test thread interrupted waiting for client transaction thread", e);
 			}
 			finally {
 				program.unlock();
 			}
-			}
 
-		}, "Lock-Thread");
+		}, "Program-Lock-Thread");
 		lockThread.start();
 
 		// wait for transaction test thread to complete
+		try {
 			txThread.join(2000);
-		assertFalse("Tx-Thread may be hung", txThread.isAlive());
+		}
+		catch (InterruptedException e) {
+			unexpectedException = new AssertException(
+				"Test thread interrupted waiting for client transaction thread", e);
+		}
 
-		Exception exc = exceptionRef.get();
-		if (exc != null) {
-			failWithException("Transaction Failure", exc);
+		if (unexpectedException != null) {
+			failWithException("Unexpected test failure", unexpectedException);
 		}
 
 	}
 
-	private class SpyDomainObjectLockedException extends MockUp<DomainObjectLockedException> {
-
-		/**
-		 * Mock/Inject constructor for DomainObjectLockedException to provide detection
-		 * of its construction via lockExceptionLatch
-		 * @param invocation
-		 * @param reason
-		 */
-		@Mock
-		public void $init(Invocation invocation, String reason) {
-			lockExceptionLatch.countDown();
-		}
-	}
 }

Ghidra/Framework/Project/src/main/java/ghidra/framework/data/DomainObjectAdapterDB.java

@@ -18,6 +18,7 @@ package ghidra.framework.data;
 import java.io.File;
 import java.io.IOException;
 import java.util.*;
+import java.util.function.Function;
 
 import db.DBConstants;
 import db.DBHandle;
@@ -82,6 +83,23 @@ public abstract class DomainObjectAdapterDB extends DomainObjectAdapter
 
 	private AbstractTransactionManager transactionMgr;
 
+	//
+	// This exception handler will sleep, allowing the startTransaction() method to ignore
+	// the exception and then try again, since it is using a while(true) loop.   This can
+	// happen if clients try to start a transaction during a long running operation, such as
+	// a program save.
+	//
+	Function<DomainObjectLockedException, Boolean> tryForeverExceptionHandler = e -> {
+		try {
+			Thread.sleep(100);
+		}
+		catch (InterruptedException ie) {
+			Msg.debug(this, "Thread interrupted while waiting for program lock: '" +
+				Thread.currentThread().getName() + "'", ie);
+		}
+		return true;
+	};
+
 	/**
 	 * Construct a new DomainObjectAdapterDB object.
 	 * If construction of this object fails, be sure to release with consumer
@@ -317,22 +335,23 @@ public abstract class DomainObjectAdapterDB extends DomainObjectAdapter
 
 	@Override
 	public int startTransaction(String description, AbortedTransactionListener listener) {
-		int id = -1;
-		while (id == -1) {
+		return startTransaction(description, listener, tryForeverExceptionHandler);
+	}
+
+	// open for testing
+	int startTransaction(String description, AbortedTransactionListener listener,
+			Function<DomainObjectLockedException, Boolean> exceptionHandler) {
+
+		while (true) {
 			try {
-				id = transactionMgr.startTransaction(this, description, listener, true);
+				return transactionMgr.startTransaction(this, description, listener, true);
 			}
 			catch (DomainObjectLockedException e) {
-				// wait for lock to be removed (e.g., Save operation)
-				try {
-					Thread.sleep(100);
-				}
-				catch (InterruptedException e1) {
-					Msg.debug(this, "Unexpected thread interrupt", e1);
+				if (!exceptionHandler.apply(e)) {
+					return -1;
 				}
 			}
 		}
-		return id;
 	}
 
 	@Override