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.
This commit is contained in:
Aikar 2019-03-26 00:31:34 -04:00
parent 9329e1383d
commit 92059ef6ba
3 changed files with 35 additions and 15 deletions

View File

@ -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 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 --- a/src/main/java/org/bukkit/command/SimpleCommandMap.java
+++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java +++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java
@@ -0,0 +0,0 @@ import java.util.Iterator; @@ -0,0 +0,0 @@ import java.util.Iterator;
@ -477,13 +477,13 @@ index f7ec2e55f..f52d91d19 100644
import org.bukkit.Server; import org.bukkit.Server;
@@ -0,0 +0,0 @@ public class SimpleCommandMap implements CommandMap { @@ -0,0 +0,0 @@ public class SimpleCommandMap implements CommandMap {
target.execute(sender, sentCommandLabel, Arrays.copyOfRange(args, 1, args.length)); target.execute(sender, sentCommandLabel, Arrays.copyOfRange(args, 1, args.length));
target.timings.stopTiming(); // Spigot } // target.timings.stopTiming(); // Spigot // Paper
} catch (CommandException ex) { } catch (CommandException ex) {
+ server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper + server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper
target.timings.stopTiming(); // Spigot //target.timings.stopTiming(); // Spigot // Paper
throw ex; throw ex;
} catch (Throwable ex) { } catch (Throwable ex) {
target.timings.stopTiming(); // Spigot //target.timings.stopTiming(); // Spigot // Paper
- throw new CommandException("Unhandled exception executing '" + commandLine + "' in " + target, ex); - throw new CommandException("Unhandled exception executing '" + commandLine + "' in " + target, ex);
+ String msg = "Unhandled exception executing '" + commandLine + "' in " + target; + String msg = "Unhandled exception executing '" + commandLine + "' in " + target;
+ server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper + server.getPluginManager().callEvent(new ServerExceptionEvent(new ServerCommandException(ex, target, sender, args))); // Paper

View File

@ -475,7 +475,7 @@ index 000000000..a5d13a1e4
+} +}
diff --git a/src/main/java/co/aikar/timings/TimingHandler.java b/src/main/java/co/aikar/timings/TimingHandler.java diff --git a/src/main/java/co/aikar/timings/TimingHandler.java b/src/main/java/co/aikar/timings/TimingHandler.java
new file mode 100644 new file mode 100644
index 000000000..2a727994a index 000000000..bb46cb633
--- /dev/null --- /dev/null
+++ b/src/main/java/co/aikar/timings/TimingHandler.java +++ b/src/main/java/co/aikar/timings/TimingHandler.java
@@ -0,0 +0,0 @@ @@ -0,0 +0,0 @@
@ -520,7 +520,7 @@ index 000000000..2a727994a
+class TimingHandler implements Timing { +class TimingHandler implements Timing {
+ +
+ private static AtomicInteger idPool = new AtomicInteger(1); + private static AtomicInteger idPool = new AtomicInteger(1);
+ static Deque<TimingHandler> TIMING_STACK = new ArrayDeque<>(); + private static Deque<TimingHandler> TIMING_STACK = new ArrayDeque<>();
+ final int id = idPool.getAndIncrement(); + final int id = idPool.getAndIncrement();
+ +
+ final TimingIdentifier identifier; + final TimingIdentifier identifier;
@ -587,10 +587,16 @@ index 000000000..2a727994a
+ +
+ public void stopTiming() { + public void stopTiming() {
+ if (enabled && timingDepth > 0 && Bukkit.isPrimaryThread() && --timingDepth == 0 && start != 0) { + if (enabled && timingDepth > 0 && Bukkit.isPrimaryThread() && --timingDepth == 0 && start != 0) {
+ TimingHandler last = TIMING_STACK.removeLast(); + TimingHandler last;
+ if (last != this) { + while ((last = TIMING_STACK.removeLast()) != this) {
+ Logger.getGlobal().log(Level.SEVERE, "TIMING_STACK_CORRUPTION - Report this to Paper! ( " + this.identifier + ":" + last +")", new Throwable()); + last.timingDepth = 0;
+ TIMING_STACK.addLast(last); // Add it back + 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()); + 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 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 --- a/src/main/java/org/bukkit/command/SimpleCommandMap.java
+++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java +++ b/src/main/java/org/bukkit/command/SimpleCommandMap.java
@@ -0,0 +0,0 @@ public class SimpleCommandMap implements CommandMap { @@ -0,0 +0,0 @@ public class SimpleCommandMap implements CommandMap {
@ -3387,8 +3393,22 @@ index 1586f6480..f7ec2e55f 100644
+ // Paper end + // Paper end
+ +
try { 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) // 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 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 deleted file mode 100644
index 1e6e7033d..000000000 index 1e6e7033d..000000000

View File

@ -1653,7 +1653,7 @@ index 646128f16..d75cc42e1 100644
private boolean isReady(final int currentTick) { 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 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 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java
+++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java
@@ -0,0 +0,0 @@ package org.bukkit.craftbukkit.scheduler; @@ -0,0 +0,0 @@ package org.bukkit.craftbukkit.scheduler;
@ -1705,13 +1705,13 @@ index 3f55381c1..f32e66010 100644
@Override @Override
public void run() { public void run() {
+ if (timings != null && isSync()) timings.startTiming(); // Paper + try (Timing ignored = timings.startTiming()) { // Paper
if (rTask != null) { if (rTask != null) {
rTask.run(); rTask.run();
} else { } else {
cTask.accept(this); cTask.accept(this);
} }
+ if (timings != null && isSync()) timings.stopTiming(); // Paper + } // Paper
} }
long getPeriod() { long getPeriod() {