From 5d78c1b6898dabf05c2134a955ca1cd698a15fd8 Mon Sep 17 00:00:00 2001
From: Hannes Braun <hannes@hannesbraun.net>
Date: Tue, 12 Sep 2023 20:28:56 +0200
Subject: [PATCH] Prevent a NullPointerException in Logfile

When stopLogPlayer is called, a running log analyzer may still be
running and will be aborted.
It may take a moment for the thread to stop. At that point logfile may
already be set to null. The LogAnalyzerThread, however, called the
"finished" callback either way. In that callback, we're trying to access
logfile (which is now null) and there's our NullPointerException.

This solution does not run any callbacks once the analyzer was aborted.
Should the abortion happen just while the callback is running, we'll use
a local reference to logfile to prevent a very unlucky
NullPointerException at that point.
---
 .../rv/comm/rcssserver/LogAnalyzerThread.java | 12 ++++++++---
 .../java/rv/comm/rcssserver/LogPlayer.java    | 20 +++++++++++++++++--
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/main/java/rv/comm/rcssserver/LogAnalyzerThread.java b/src/main/java/rv/comm/rcssserver/LogAnalyzerThread.java
index 25b881b..5c45348 100644
--- a/src/main/java/rv/comm/rcssserver/LogAnalyzerThread.java
+++ b/src/main/java/rv/comm/rcssserver/LogAnalyzerThread.java
@@ -87,7 +87,9 @@ public void run()
 		}
 		processFrame();
 
-		callback.finished(logfile.getNumFrames());
+		if (!aborted) {
+			callback.finished(logfile.getNumFrames());
+		}
 
 		try {
 			logfile.close();
@@ -127,7 +129,9 @@ private void processGoals()
 			int frame = logfile.getCurrentFrame();
 			int goalWindowFrames = Math.round((1 / stepSize) * LogPlayer.GOAL_WINDOW_SECONDS);
 			int viewFrame = Math.max(0, frame - goalWindowFrames);
-			callback.goalFound(new Goal(frame, viewFrame, scoringTeam));
+			if (!aborted) {
+				callback.goalFound(new Goal(frame, viewFrame, scoringTeam));
+			}
 		}
 
 		lastScoreLeft = scoreLeft;
@@ -155,7 +159,9 @@ private void processStepSize()
 				// estimate total number of frames
 				float halfTime = world.getGameState().getHalfTime();
 				int numFrames = Math.round(((1 / stepSize) * halfTime) + numPauseFrames);
-				callback.stepSizeFound(stepSize, numFrames);
+				if (!aborted) {
+					callback.stepSizeFound(stepSize, numFrames);
+				}
 				this.stepSize = stepSize;
 			}
 
diff --git a/src/main/java/rv/comm/rcssserver/LogPlayer.java b/src/main/java/rv/comm/rcssserver/LogPlayer.java
index fbfd20d..fa9a156 100644
--- a/src/main/java/rv/comm/rcssserver/LogPlayer.java
+++ b/src/main/java/rv/comm/rcssserver/LogPlayer.java
@@ -411,17 +411,29 @@ private void startAnalyzerThread(File file)
 			logAnalyzer.abort();
 		}
 		logAnalyzer = new LogAnalyzerThread(file, new LogAnalyzerThread.ResultCallback() {
+			/*
+			  A reference to the logfile reader is stored locally in the beginning of each callback.
+			  This prevents a NullPointerException when logfile is suddenly set to null in another thread.
+			 */
 			@Override
 			public void stepSizeFound(float stepSize, int numFrames)
 			{
+				ILogfileReader logfileReader = logfile;
+				if (logfile == null) {
+					return;
+				}
 				foundStepSize = true;
 				SECONDS_PER_FRAME = stepSize;
-				logfile.setNumFrames(numFrames);
+				logfileReader.setNumFrames(numFrames);
 			}
 
 			@Override
 			public void goalFound(Goal goal)
 			{
+				ILogfileReader logfileReader = logfile;
+				if (logfile == null) {
+					return;
+				}
 				goals.add(goal);
 				analyzedFrames = goal.frame;
 				stateChanged();
@@ -430,8 +442,12 @@ public void goalFound(Goal goal)
 			@Override
 			public void finished(int numFrames)
 			{
+				ILogfileReader logfileReader = logfile;
+				if (logfile == null) {
+					return;
+				}
 				LogPlayer.this.logAnalyzed = true;
-				logfile.setNumFrames(numFrames);
+				logfileReader.setNumFrames(numFrames);
 				analyzedFrames = numFrames;
 				stateChanged();
 			}