From d4c49a7539de8ca60096259cb4ac1593b478b08b Mon Sep 17 00:00:00 2001 From: wizjany Date: Sat, 11 May 2019 14:21:07 -0400 Subject: [PATCH] Clean up task monitoring, cancelling, exception handling etc. 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. --- config/checkstyle/import-control.xml | 2 +- .../java/com/sk89q/worldguard/WorldGuard.java | 11 +++--- .../commands/WorldGuardCommands.java | 39 ++++++++++--------- .../worldguard/protection/flags/Flags.java | 6 --- .../util/WorldGuardExceptionConverter.java | 32 +++++++-------- .../util/profiler/SamplerBuilder.java | 9 ++++- 6 files changed, 49 insertions(+), 50 deletions(-) diff --git a/config/checkstyle/import-control.xml b/config/checkstyle/import-control.xml index 0ff2e1ba0..2b0e48ce5 100644 --- a/config/checkstyle/import-control.xml +++ b/config/checkstyle/import-control.xml @@ -17,11 +17,11 @@ + - diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/WorldGuard.java b/worldguard-core/src/main/java/com/sk89q/worldguard/WorldGuard.java index 9cc36de34..3c1b34cc7 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/WorldGuard.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/WorldGuard.java @@ -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(); @@ -192,7 +192,7 @@ 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> tasks = supervisor.getTasks(); if (!tasks.isEmpty()) { @@ -200,16 +200,15 @@ public void disable() { 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(); diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/commands/WorldGuardCommands.java b/worldguard-core/src/main/java/com/sk89q/worldguard/commands/WorldGuardCommands.java index 55843d376..9837563b7 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/commands/WorldGuardCommands.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/commands/WorldGuardCommands.java @@ -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; @@ -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) { @@ -263,26 +271,19 @@ public void flushStates(CommandContext args, Actor sender) throws CommandExcepti public void listRunningTasks(CommandContext args, Actor sender) throws CommandException { List> 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()); } } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java index 39d86a341..b9a1b3240 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/protection/flags/Flags.java @@ -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; @@ -188,11 +187,6 @@ private static > 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. * diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/util/WorldGuardExceptionConverter.java b/worldguard-core/src/main/java/com/sk89q/worldguard/util/WorldGuardExceptionConverter.java index 6f71f8335..92c5a062f 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/util/WorldGuardExceptionConverter.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/util/WorldGuardExceptionConverter.java @@ -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; @@ -39,11 +39,9 @@ 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 @@ -51,46 +49,46 @@ 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); } } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/util/profiler/SamplerBuilder.java b/worldguard-core/src/main/java/com/sk89q/worldguard/util/profiler/SamplerBuilder.java index adabd1fdc..093ee2934 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/util/profiler/SamplerBuilder.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/util/profiler/SamplerBuilder.java @@ -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; @@ -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 { @@ -129,7 +136,7 @@ public synchronized void run() { } } catch (Throwable t) { future.setException(t); - cancel(); + super.cancel(); } }