Skip to content

Commit

Permalink
Clean up task monitoring, cancelling, exception handling etc.
Browse files Browse the repository at this point in the history
Fixes a few issues such as /wg stopprofile leaving a pseudo-cancelled
task in the supervisor, delaying server shutdowns until profiles finish,
not forwarding exceptions that occur during tasks running correctly, etc.
  • Loading branch information
wizjany committed May 11, 2019
1 parent 2e2be70 commit d4c49a7
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 50 deletions.
2 changes: 1 addition & 1 deletion config/checkstyle/import-control.xml
Expand Up @@ -17,11 +17,11 @@
<allow pkg="org.flywaydb"/>
<allow pkg="org.yaml"/>
<allow pkg="org.json"/>
<allow pkg="org.enginehub.piston"/>

<subpackage name="bukkit">
<allow pkg="org.bukkit"/>
<allow pkg="org.bstats.bukkit"/>
<allow pkg="net.minecraft.server"/>
</subpackage>

<subpackage name="sponge">
Expand Down
Expand Up @@ -65,7 +65,7 @@ public final class WorldGuard {
private ProfileCache profileCache;
private ProfileService profileService;
private ListeningExecutorService executorService;
private WorldGuardExceptionConverter exceptionConverter = new WorldGuardExceptionConverter(this);
private WorldGuardExceptionConverter exceptionConverter = new WorldGuardExceptionConverter();

static {
Flags.registerAll();
Expand Down Expand Up @@ -192,24 +192,23 @@ public void disable() {
executorService.shutdown();

try {
logger.log(Level.INFO, "Shutting down executor and waiting for any pending tasks...");
logger.log(Level.INFO, "Shutting down executor and cancelling any pending tasks...");

List<Task<?>> tasks = supervisor.getTasks();
if (!tasks.isEmpty()) {
StringBuilder builder = new StringBuilder("Known tasks:");
for (Task<?> task : tasks) {
builder.append("\n");
builder.append(task.getName());
task.cancel(true);
}
logger.log(Level.INFO, builder.toString());
}

Futures.successfulAsList(tasks).get();
executorService.awaitTermination(Integer.MAX_VALUE, TimeUnit.DAYS);
//Futures.successfulAsList(tasks).get();
executorService.awaitTermination(5, TimeUnit.SECONDS);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
} catch (ExecutionException e) {
logger.log(Level.WARNING, "Some tasks failed while waiting for remaining tasks to finish", e);
}

platform.unload();
Expand Down
Expand Up @@ -33,6 +33,10 @@
import com.sk89q.worldedit.extension.platform.Actor;
import com.sk89q.worldedit.extension.platform.Capability;
import com.sk89q.worldedit.util.auth.AuthorizationException;
import com.sk89q.worldedit.util.formatting.component.MessageBox;
import com.sk89q.worldedit.util.formatting.component.TextComponentProducer;
import com.sk89q.worldedit.util.formatting.text.TextComponent;
import com.sk89q.worldedit.util.formatting.text.event.ClickEvent;
import com.sk89q.worldedit.util.formatting.text.format.TextColor;
import com.sk89q.worldedit.util.paste.ActorCallbackPaste;
import com.sk89q.worldedit.util.report.ReportList;
Expand Down Expand Up @@ -191,11 +195,15 @@ public void profile(final CommandContext args, final Actor sender) throws Comman
sampler = activeSampler = builder.start();
}

sender.print(TextComponent.of("Starting CPU profiling. Use ", TextColor.LIGHT_PURPLE)
.append(TextComponent.of("/wg stopprofle", TextColor.AQUA)
.clickEvent(ClickEvent.of(ClickEvent.Action.RUN_COMMAND, "/wg stopprofile")))
.append(TextComponent.of(" to stop profling.", TextColor.LIGHT_PURPLE)));
AsyncCommandHelper.wrap(sampler.getFuture(), worldGuard.getSupervisor(), sender, worldGuard.getExceptionConverter())
.formatUsing(minutes)
.registerWithSupervisor("Running CPU profiler for %d minute(s)...")
.sendMessageAfterDelay("(Please wait... profiling for %d minute(s)...)")
.thenTellErrorsOnly("CPU profiling failed.");
.thenTellErrorsOnly("CPU profiling failed");

sampler.getFuture().addListener(() -> {
synchronized (WorldGuardCommands.this) {
Expand Down Expand Up @@ -263,26 +271,19 @@ public void flushStates(CommandContext args, Actor sender) throws CommandExcepti
public void listRunningTasks(CommandContext args, Actor sender) throws CommandException {
List<Task<?>> tasks = WorldGuard.getInstance().getSupervisor().getTasks();

if (!tasks.isEmpty()) {
if (tasks.isEmpty()) {
sender.print("There are currently no running tasks.");
} else {
tasks.sort(new TaskStateComparator());
StringBuilder builder = new StringBuilder();
builder.append(TextColor.GRAY);
builder.append("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550");
builder.append(" Running tasks ");
builder.append("\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550\u2550");
builder.append("\n").append(TextColor.GRAY).append("Note: Some 'running' tasks may be waiting to be start.");
for (Task task : tasks) {
builder.append("\n");
builder.append(TextColor.BLUE).append("(").append(task.getState().name()).append(") ");
builder.append(TextColor.YELLOW);
builder.append(CommandUtils.getOwnerName(task.getOwner()));
builder.append(": ");
builder.append(TextColor.WHITE);
builder.append(task.getName());
MessageBox builder = new MessageBox("Running Tasks", new TextComponentProducer());
builder.append(TextComponent.of("Note: Some 'running' tasks may be waiting to be start.", TextColor.GRAY));
for (Task<?> task : tasks) {
builder.append(TextComponent.newline());
builder.append(TextComponent.of("(" + task.getState().name() + ") ", TextColor.BLUE));
builder.append(TextComponent.of(CommandUtils.getOwnerName(task.getOwner()) + ": ", TextColor.YELLOW));
builder.append(TextComponent.of(task.getName(), TextColor.WHITE));
}
sender.printRaw(builder.toString());
} else {
sender.print("There are currently no running tasks.");
sender.print(builder.create());
}
}

Expand Down
Expand Up @@ -31,7 +31,6 @@
import com.sk89q.worldguard.protection.flags.registry.FlagConflictException;
import com.sk89q.worldguard.protection.flags.registry.FlagRegistry;

import javax.annotation.Nullable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -188,11 +187,6 @@ private static <T extends Flag<?>> T register(final T flag) throws FlagConflictE
return flag;
}

@Nullable
public static Flag<?> get(final String id) {
return WorldGuard.getInstance().getFlagRegistry().get(id);
}

/**
* Try to match the flag with the given ID using a fuzzy name match.
*
Expand Down
Expand Up @@ -19,16 +19,16 @@

package com.sk89q.worldguard.util;

import static com.google.common.base.Preconditions.checkNotNull;

import com.sk89q.minecraft.util.commands.CommandException;
import com.google.common.collect.ImmutableList;
import com.sk89q.worldedit.WorldEditException;
import com.sk89q.worldedit.internal.command.exception.ExceptionConverterHelper;
import com.sk89q.worldedit.internal.command.exception.ExceptionMatch;
import com.sk89q.worldedit.util.auth.AuthorizationException;
import com.sk89q.worldedit.util.formatting.text.TextComponent;
import com.sk89q.worldguard.WorldGuard;
import com.sk89q.worldguard.protection.managers.storage.StorageException;
import com.sk89q.worldguard.protection.util.UnresolvedNamesException;
import org.enginehub.piston.exception.CommandException;

import java.util.concurrent.CancellationException;
import java.util.concurrent.RejectedExecutionException;
Expand All @@ -39,58 +39,56 @@
public class WorldGuardExceptionConverter extends ExceptionConverterHelper {

private static final Pattern numberFormat = Pattern.compile("^For input string: \"(.*)\"$");
private final WorldGuard worldGuard;

public WorldGuardExceptionConverter(WorldGuard worldGuard) {
checkNotNull(worldGuard);
this.worldGuard = worldGuard;
private CommandException newCommandException(String message, Throwable cause) {
return new CommandException(TextComponent.of(String.valueOf(message)), cause, ImmutableList.of());
}

@ExceptionMatch
public void convert(NumberFormatException e) throws CommandException {
final Matcher matcher = numberFormat.matcher(e.getMessage());

if (matcher.matches()) {
throw new CommandException("Number expected; string \"" + matcher.group(1)
+ "\" given.");
throw newCommandException("Number expected; string \"" + matcher.group(1)
+ "\" given.", e);
} else {
throw new CommandException("Number expected; string given.");
throw newCommandException("Number expected; string given.", e);
}
}

@ExceptionMatch
public void convert(StorageException e) throws CommandException {
WorldGuard.logger.log(Level.WARNING, "Error loading/saving regions", e);
throw new CommandException("Region data could not be loaded/saved: " + e.getMessage());
throw newCommandException("Region data could not be loaded/saved: " + e.getMessage(), e);
}

@ExceptionMatch
public void convert(RejectedExecutionException e) throws CommandException {
throw new CommandException("There are currently too many tasks queued to add yours. Use /wg running to list queued and running tasks.", e);
throw newCommandException("There are currently too many tasks queued to add yours. Use /wg running to list queued and running tasks.", e);
}

@ExceptionMatch
public void convert(CancellationException e) throws CommandException {
throw new CommandException("WorldGuard: Task was cancelled.", e);
throw newCommandException("Task was cancelled.", e);
}

@ExceptionMatch
public void convert(InterruptedException e) throws CommandException {
throw new CommandException("WorldGuard: Task was interrupted.", e);
throw newCommandException("Task was interrupted.", e);
}

@ExceptionMatch
public void convert(WorldEditException e) throws CommandException {
throw new CommandException(e.getMessage(), e);
throw newCommandException(e.getMessage(), e);
}

@ExceptionMatch
public void convert(UnresolvedNamesException e) throws CommandException {
throw new CommandException(e.getMessage(), e);
throw newCommandException(e.getMessage(), e);
}

@ExceptionMatch
public void convert(AuthorizationException e) throws CommandException {
throw new CommandException("You don't have permission to do that.", e);
throw newCommandException("You don't have permission to do that.", e);
}
}
Expand Up @@ -33,6 +33,7 @@
import java.util.Timer;
import java.util.TimerTask;
import java.util.TreeMap;
import java.util.concurrent.CancellationException;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;

Expand Down Expand Up @@ -108,6 +109,12 @@ private StackNode getNode(String name) {
return node;
}

@Override
public boolean cancel() {
future.setException(new CancellationException());
return super.cancel();
}

@Override
public synchronized void run() {
try {
Expand All @@ -129,7 +136,7 @@ public synchronized void run() {
}
} catch (Throwable t) {
future.setException(t);
cancel();
super.cancel();
}
}

Expand Down

0 comments on commit d4c49a7

Please sign in to comment.