Skip to content

Commit

Permalink
Prevent a NullPointerException in Logfile
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hannesbraun committed Sep 12, 2023
1 parent 8d440fb commit 5d78c1b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
12 changes: 9 additions & 3 deletions src/main/java/rv/comm/rcssserver/LogAnalyzerThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ public void run()
}
processFrame();

callback.finished(logfile.getNumFrames());
if (!aborted) {
callback.finished(logfile.getNumFrames());
}

try {
logfile.close();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
20 changes: 18 additions & 2 deletions src/main/java/rv/comm/rcssserver/LogPlayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
Expand Down

0 comments on commit 5d78c1b

Please sign in to comment.