Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tasks leaking after match end #1383

Merged
merged 5 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 16 additions & 21 deletions core/src/main/java/tc/oc/pgm/listeners/MatchAnnouncer.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ public void onMatchLoad(final MatchLoadEvent event) {
final Match match = event.getMatch();
match
.getExecutor(MatchScope.LOADED)
.scheduleWithFixedDelay(
() -> match.getPlayers().forEach(this::sendCurrentlyPlaying), 0, 5, TimeUnit.MINUTES);
.scheduleWithFixedDelay(() -> sendCurrentlyPlaying(match), 0, 5, TimeUnit.MINUTES);
}

@EventHandler(priority = EventPriority.MONITOR)
Expand Down Expand Up @@ -83,12 +82,11 @@ public void onMatchEnd(final MatchFinishEvent event) {
title = translatable("broadcast.gameOver");
} else {
if (singleWinner) {
title =
translatable(
Iterables.getOnlyElement(winners).isNamePlural()
? "broadcast.gameOver.teamWinners"
: "broadcast.gameOver.teamWinner",
TextFormatter.nameList(winners, NameStyle.FANCY, NamedTextColor.WHITE));
title = translatable(
Iterables.getOnlyElement(winners).isNamePlural()
? "broadcast.gameOver.teamWinners"
: "broadcast.gameOver.teamWinner",
TextFormatter.nameList(winners, NameStyle.FANCY, NamedTextColor.WHITE));
}

// Use stream here instead of #contains to avoid unchecked cast
Expand Down Expand Up @@ -156,13 +154,11 @@ public void sendWelcomeMessage(MatchPlayer viewer, List<Component> extraLines) {

Collection<Contributor> authors = mapInfo.getAuthors();
if (!authors.isEmpty()) {
viewer.sendMessage(
space()
.append(
translatable(
"misc.createdBy",
NamedTextColor.GRAY,
TextFormatter.nameList(authors, NameStyle.FANCY, NamedTextColor.GRAY))));
viewer.sendMessage(space()
.append(translatable(
"misc.createdBy",
NamedTextColor.GRAY,
TextFormatter.nameList(authors, NameStyle.FANCY, NamedTextColor.GRAY))));
}

// Send extra info from other plugins
Expand All @@ -173,11 +169,10 @@ public void sendWelcomeMessage(MatchPlayer viewer, List<Component> extraLines) {
viewer.sendMessage(TextFormatter.horizontalLine(NamedTextColor.WHITE, 200));
}

private void sendCurrentlyPlaying(MatchPlayer viewer) {
viewer.sendMessage(
translatable(
"misc.playing",
NamedTextColor.DARK_PURPLE,
viewer.getMatch().getMap().getStyledName(MapNameStyle.COLOR_WITH_AUTHORS)));
private void sendCurrentlyPlaying(Match match) {
match.sendMessage(translatable(
"misc.playing",
NamedTextColor.DARK_PURPLE,
match.getMap().getStyledName(MapNameStyle.COLOR_WITH_AUTHORS)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.jetbrains.annotations.NotNull;
import tc.oc.pgm.util.TimeUtils;

/** An executor service that is backed by a runnable executor. */
Expand Down Expand Up @@ -64,7 +65,7 @@ public List<Runnable> shutdownNow() {
if (!isShutdown()) shutdown();
List<Runnable> pending = ImmutableList.copyOf(tasks);

for (Task<?> task : tasks) {
for (var task : tasks) {
task.cancel(true);
}

Expand Down Expand Up @@ -234,49 +235,35 @@ public void run() {

@Override
public boolean complete(V value) {
return complete(value, null);
return !periodic && cleanup(super.complete(value));
}

@Override
public boolean completeExceptionally(Throwable ex) {
return complete(null, ex);
return cleanup(super.completeExceptionally(ex));
}

@Override
public boolean cancel(boolean mayInterruptIfRunning) {
if (super.cancel(mayInterruptIfRunning)) {
tasks.remove(this);
if (terminated != null) {
terminated.countDown();
}
cancelTask(taskId);
return true;
}

return false;
return cleanup(super.cancel(mayInterruptIfRunning));
}

@Override
public long getDelay(TimeUnit unit) {
public long getDelay(@NotNull TimeUnit unit) {
return 0;
}

@Override
public int compareTo(Delayed o) {
if (!(o instanceof Task)) return 0;
return isDone() ? 1 : 0;
public int compareTo(@NotNull Delayed other) {
if (!(other instanceof Task<?> ot)) return -1;
return Integer.compare(taskId, ot.taskId);
}

private boolean complete(V value, Throwable ex) {
boolean done = false;

if (!periodic && value != null) {
done = super.complete(value);
} else if (ex != null) {
done = super.completeExceptionally(ex);
}

return !done || cancel(true);
private boolean cleanup(boolean triggered) {
if (terminated != null) terminated.countDown();
cancelTask(taskId);
tasks.remove(this);
return triggered;
}
}
}