From e055103d3dbafe8dabc54aa38f8e84bec9d15167 Mon Sep 17 00:00:00 2001 From: Aikar Date: Tue, 26 Mar 2019 00:31:34 -0400 Subject: [PATCH] Improve Timings stack protection more Ensures in more places that exceptions will not corrupt the Timings stack. Timings will now better report stack corruption and auto repair itself too. --- Spigot-API-Patches/0004-Timings-v2.patch | 42 ++++++++++++++----- .../0017-Add-exception-reporting-event.patch | 10 ++--- Spigot-Server-Patches/0009-Timings-v2.patch | 8 ++-- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Spigot-API-Patches/0004-Timings-v2.patch b/Spigot-API-Patches/0004-Timings-v2.patch index 8b700dea58..046f514601 100644 --- a/Spigot-API-Patches/0004-Timings-v2.patch +++ b/Spigot-API-Patches/0004-Timings-v2.patch @@ -1,4 +1,4 @@ -From f7754e7d136f3f3a688bc358e4e53cba5f1d44fb Mon Sep 17 00:00:00 2001 +From 7a646006087774fb9c2558e741c74f3e3e0a6819 Mon Sep 17 00:00:00 2001 From: Aikar Date: Mon, 29 Feb 2016 18:48:17 -0600 Subject: [PATCH] Timings v2 @@ -475,10 +475,10 @@ index 000000000..a5d13a1e4 +} diff --git a/src/main/java/co/aikar/timings/TimingHandler.java b/src/main/java/co/aikar/timings/TimingHandler.java new file mode 100644 -index 000000000..2a727994a +index 000000000..bb46cb633 --- /dev/null +++ b/src/main/java/co/aikar/timings/TimingHandler.java -@@ -0,0 +1,202 @@ +@@ -0,0 +1,208 @@ +/* + * This file is licensed under the MIT License (MIT). + * @@ -520,7 +520,7 @@ index 000000000..2a727994a +class TimingHandler implements Timing { + + private static AtomicInteger idPool = new AtomicInteger(1); -+ static Deque TIMING_STACK = new ArrayDeque<>(); ++ private static Deque TIMING_STACK = new ArrayDeque<>(); + final int id = idPool.getAndIncrement(); + + final TimingIdentifier identifier; @@ -587,10 +587,16 @@ index 000000000..2a727994a + + public void stopTiming() { + if (enabled && timingDepth > 0 && Bukkit.isPrimaryThread() && --timingDepth == 0 && start != 0) { -+ TimingHandler last = TIMING_STACK.removeLast(); -+ if (last != this) { -+ Logger.getGlobal().log(Level.SEVERE, "TIMING_STACK_CORRUPTION - Report this to Paper! ( " + this.identifier + ":" + last +")", new Throwable()); -+ TIMING_STACK.addLast(last); // Add it back ++ TimingHandler last; ++ while ((last = TIMING_STACK.removeLast()) != this) { ++ last.timingDepth = 0; ++ String reportTo; ++ if ("minecraft".equals(last.identifier.group)) { ++ reportTo = "Paper! This is a potential bug in Paper"; ++ } else { ++ reportTo = "the plugin " + last.identifier.group + "(Look for errors above this in the logs)"; ++ } ++ Logger.getGlobal().log(Level.SEVERE, "TIMING_STACK_CORRUPTION - Report this to " + reportTo + " (" + last.identifier +" did not stopTiming)", new Throwable()); + } + addDiff(System.nanoTime() - start, TIMING_STACK.peekLast()); + @@ -3356,7 +3362,7 @@ index 000000000..ca1893e9f + +} diff --git a/src/main/java/org/bukkit/command/SimpleCommandMap.java b/src/main/java/org/bukkit/command/SimpleCommandMap.java -index 1586f6480..f7ec2e55f 100644 +index 1586f6480..6a786b8c5 100644 --- a/src/main/java/org/bukkit/command/SimpleCommandMap.java +++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java @@ -31,7 +31,7 @@ public class SimpleCommandMap implements CommandMap { @@ -3376,7 +3382,7 @@ index 1586f6480..f7ec2e55f 100644 label = label.toLowerCase(java.util.Locale.ENGLISH).trim(); fallbackPrefix = fallbackPrefix.toLowerCase(java.util.Locale.ENGLISH).trim(); boolean registered = register(label, command, false, fallbackPrefix); -@@ -135,6 +136,12 @@ public class SimpleCommandMap implements CommandMap { +@@ -135,16 +136,22 @@ public class SimpleCommandMap implements CommandMap { return false; } @@ -3387,8 +3393,22 @@ index 1586f6480..f7ec2e55f 100644 + // Paper end + try { - target.timings.startTiming(); // Spigot +- target.timings.startTiming(); // Spigot ++ try (co.aikar.timings.Timing ignored = target.timings.startTiming()) { // Paper - use try with resources // Note: we don't return the result of target.execute as thats success / failure, we return handled (true) or not handled (false) + target.execute(sender, sentCommandLabel, Arrays.copyOfRange(args, 1, args.length)); +- target.timings.stopTiming(); // Spigot ++ } // target.timings.stopTiming(); // Spigot // Paper + } catch (CommandException ex) { +- target.timings.stopTiming(); // Spigot ++ //target.timings.stopTiming(); // Spigot // Paper + throw ex; + } catch (Throwable ex) { +- target.timings.stopTiming(); // Spigot ++ //target.timings.stopTiming(); // Spigot // Paper + throw new CommandException("Unhandled exception executing '" + commandLine + "' in " + target, ex); + } + diff --git a/src/main/java/org/bukkit/command/defaults/TimingsCommand.java b/src/main/java/org/bukkit/command/defaults/TimingsCommand.java deleted file mode 100644 index 1e6e7033d..000000000 diff --git a/Spigot-API-Patches/0017-Add-exception-reporting-event.patch b/Spigot-API-Patches/0017-Add-exception-reporting-event.patch index fcb3ee95d2..24dc54319c 100644 --- a/Spigot-API-Patches/0017-Add-exception-reporting-event.patch +++ b/Spigot-API-Patches/0017-Add-exception-reporting-event.patch @@ -1,4 +1,4 @@ -From 6c5e4653f78507a8553b8e9f915449a0ca3aa50f Mon Sep 17 00:00:00 2001 +From e1e05e05e79b9ba25d403b5648b57c59b1c622cf Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 29 Feb 2016 20:24:35 -0600 Subject: [PATCH] Add exception reporting event @@ -462,7 +462,7 @@ index 000000000..5582999fe + } +} diff --git a/src/main/java/org/bukkit/command/SimpleCommandMap.java b/src/main/java/org/bukkit/command/SimpleCommandMap.java -index f7ec2e55f..f52d91d19 100644 +index 6a786b8c5..682d27f94 100644 --- a/src/main/java/org/bukkit/command/SimpleCommandMap.java +++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java @@ -9,6 +9,9 @@ import java.util.Iterator; @@ -477,13 +477,13 @@ index f7ec2e55f..f52d91d19 100644 import org.bukkit.Server; @@ -148,11 +151,14 @@ public class SimpleCommandMap implements CommandMap { target.execute(sender, sentCommandLabel, Arrays.copyOfRange(args, 1, args.length)); - target.timings.stopTiming(); // Spigot + } // target.timings.stopTiming(); // Spigot // Paper } catch (CommandException ex) { + server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper - target.timings.stopTiming(); // Spigot + //target.timings.stopTiming(); // Spigot // Paper throw ex; } catch (Throwable ex) { - target.timings.stopTiming(); // Spigot + //target.timings.stopTiming(); // Spigot // Paper - throw new CommandException("Unhandled exception executing '" + commandLine + "' in " + target, ex); + String msg = "Unhandled exception executing '" + commandLine + "' in " + target; + server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper diff --git a/Spigot-Server-Patches/0009-Timings-v2.patch b/Spigot-Server-Patches/0009-Timings-v2.patch index 4526d06bde..40f18dbf83 100644 --- a/Spigot-Server-Patches/0009-Timings-v2.patch +++ b/Spigot-Server-Patches/0009-Timings-v2.patch @@ -1,4 +1,4 @@ -From c4854e60305b3205de80651deb632f7a99dcd8ac Mon Sep 17 00:00:00 2001 +From f2743e61a346316b3627c75f8d6beb3c30a8cd75 Mon Sep 17 00:00:00 2001 From: Aikar Date: Thu, 3 Mar 2016 04:00:11 -0600 Subject: [PATCH] Timings v2 @@ -1653,7 +1653,7 @@ index 646128f16..d75cc42e1 100644 private boolean isReady(final int currentTick) { diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java -index 3f55381c1..f32e66010 100644 +index 3f55381c1..17ba052f8 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java @@ -2,8 +2,8 @@ package org.bukkit.craftbukkit.scheduler; @@ -1705,13 +1705,13 @@ index 3f55381c1..f32e66010 100644 @Override public void run() { -+ if (timings != null && isSync()) timings.startTiming(); // Paper ++ try (Timing ignored = timings.startTiming()) { // Paper if (rTask != null) { rTask.run(); } else { cTask.accept(this); } -+ if (timings != null && isSync()) timings.stopTiming(); // Paper ++ } // Paper } long getPeriod() {