From b04304be4bf439240b2b2ab758d50d692e10fe22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= <laeubi@laeubi-soft.de>
Date: Tue, 27 Feb 2024 11:00:13 +0100
Subject: [PATCH] Fix NPE in TestRunSession#toString and use AtomicReference

currently if one calls TestRunSession#toString (e.g. in a debugger) a
NPE can occur if the session is not yet started and the fStartTime is
null. Also it is possible that fStartTime is concurrently set.

This checks for this case adjust the message accordingly and also
replace the volatile field with an AtomicReference to ensure the start
time is only ever set by the first thread, alongside with that an
unnecessary synchronized is removed from the method
addTestSessionListener as ListenerList is already synchronized.
---
 .../internal/model/TestRunSession.java        | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/debug/org.eclipse.unittest.ui/src/org/eclipse/unittest/internal/model/TestRunSession.java b/debug/org.eclipse.unittest.ui/src/org/eclipse/unittest/internal/model/TestRunSession.java
index 333ee659d2a..12d6ca1049a 100644
--- a/debug/org.eclipse.unittest.ui/src/org/eclipse/unittest/internal/model/TestRunSession.java
+++ b/debug/org.eclipse.unittest.ui/src/org/eclipse/unittest/internal/model/TestRunSession.java
@@ -23,6 +23,7 @@
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.unittest.internal.UnitTestPlugin;
 import org.eclipse.unittest.internal.launcher.TestViewSupportRegistry;
@@ -78,7 +79,7 @@ public class TestRunSession extends TestSuiteElement implements ITestRunSession,
 	 */
 	private HashMap<String, TestElement> fIdToTest;
 
-	volatile Instant fStartTime;
+	private final AtomicReference<Instant> fStartTime = new AtomicReference<>();
 	volatile Integer fPredefinedTestCount;
 
 	volatile boolean fIsAborted;
@@ -105,7 +106,7 @@ public TestRunSession(String testRunName, Instant startTime, ILaunchConfiguratio
 		fIdToTest = new HashMap<>();
 
 		fTestRunnerClient = null;
-		fStartTime = startTime;
+		fStartTime.set(startTime);
 
 		fSessionListeners = new ListenerList<>();
 	}
@@ -254,7 +255,7 @@ public int getCurrentIgnoredCount() {
 	 * @return an {@link Instant} object indicating a test run session start time
 	 */
 	public Instant getStartTime() {
-		return fStartTime;
+		return fStartTime.get();
 	}
 
 	/**
@@ -273,7 +274,7 @@ public boolean isStopped() {
 	 *
 	 * @param listener an {@link ITestSessionListener} object
 	 */
-	public synchronized void addTestSessionListener(ITestSessionListener listener) {
+	public void addTestSessionListener(ITestSessionListener listener) {
 		fSessionListeners.add(listener);
 	}
 
@@ -381,7 +382,8 @@ private class TestSessionNotifier {
 		 * @param testCount number of tests in this run
 		 */
 		public void testRunStarted(Integer testCount) {
-			fStartTime = Instant.now();
+			// only update if not already set!
+			fStartTime.compareAndSet(null, Instant.now());
 			fPredefinedTestCount = testCount;
 
 			for (ITestSessionListener listener : fSessionListeners) {
@@ -590,7 +592,12 @@ private void addFailures(Collection<TestElement> failures, TestElement testEleme
 
 	@Override
 	public String toString() {
-		return fTestRunName + " " + DateFormat.getDateTimeInstance().format(new Date(fStartTime.toEpochMilli())); //$NON-NLS-1$
+		Instant startTime = getStartTime();
+		if (startTime == null) {
+			return fTestRunName + " (not started)"; //$NON-NLS-1$
+		} else {
+			return fTestRunName + " " + DateFormat.getDateTimeInstance().format(new Date(startTime.toEpochMilli())); //$NON-NLS-1$
+		}
 	}
 
 	@Override