From d01d2d8cc5be57338f6896d315428d21b7c1629a Mon Sep 17 00:00:00 2001 From: Aikar Date: Mon, 3 Sep 2018 22:36:41 -0400 Subject: [PATCH] Fix concurrency issues in DataFixers We are seeing issues with DataFixers being not thread safe in async chunks and even in some spigot packet sending code. There are a few more global objects that are mutated that need to be synchronized to be safe for use over multiple threads. There may be more cases, but these are extremely obvious ones. --- ...Fix-concurrency-issues-in-DataFixers.patch | 124 ++++++++++++++++++ scripts/importmcdev.sh | 6 +- 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 Spigot-Server-Patches/0360-Fix-concurrency-issues-in-DataFixers.patch diff --git a/Spigot-Server-Patches/0360-Fix-concurrency-issues-in-DataFixers.patch b/Spigot-Server-Patches/0360-Fix-concurrency-issues-in-DataFixers.patch new file mode 100644 index 0000000000..182ead74f9 --- /dev/null +++ b/Spigot-Server-Patches/0360-Fix-concurrency-issues-in-DataFixers.patch @@ -0,0 +1,124 @@ +From 2f22a5cc15f69a04dd205f4a24c8cf280f0f4a57 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Mon, 3 Sep 2018 22:18:38 -0400 +Subject: [PATCH] Fix concurrency issues in DataFixers + +We are seeing issues with DataFixers being not thread safe in async chunks +and even in some spigot packet sending code. + +There are a few more global objects that are mutated that need to +be synchronized to be safe for use over multiple threads. + +There may be more cases, but these are extremely obvious ones. + +diff --git a/src/main/java/com/mojang/datafixers/DataFixerUpper.java b/src/main/java/com/mojang/datafixers/DataFixerUpper.java +index fb2c380f8a..9299cbbdbd 100644 +--- a/src/main/java/com/mojang/datafixers/DataFixerUpper.java ++++ b/src/main/java/com/mojang/datafixers/DataFixerUpper.java +@@ -125,7 +125,7 @@ public class DataFixerUpper implements DataFixer { + final int expandedDataVersion = DataFixUtils.makeKey(dataVersion); + + final long key = (long) expandedVersion << 32 | expandedDataVersion; +- if (!rules.containsKey(key)) { ++ rules.computeIfAbsent(key, k -> { // Paper + final List rules = Lists.newArrayList(); + for (final DataFix fix : globalList) { + final int fixVersion = fix.getVersionKey(); +@@ -137,8 +137,8 @@ public class DataFixerUpper implements DataFixer { + rules.add(fixRule); + } + } +- this.rules.put(key, TypeRewriteRule.seq(rules)); +- } ++ return TypeRewriteRule.seq(rules); // Paper ++ }); // Paper + return rules.get(key); + } + +diff --git a/src/main/java/com/mojang/datafixers/schemas/Schema.java b/src/main/java/com/mojang/datafixers/schemas/Schema.java +index 7c67d989e0..45f3ef5957 100644 +--- a/src/main/java/com/mojang/datafixers/schemas/Schema.java ++++ b/src/main/java/com/mojang/datafixers/schemas/Schema.java +@@ -20,8 +20,8 @@ import java.util.function.Function; + import java.util.function.Supplier; + + public class Schema { +- protected final Object2IntMap RECURSIVE_TYPES = new Object2IntOpenHashMap<>(); +- private final Map> TYPE_TEMPLATES = Maps.newHashMap(); ++ protected final Object2IntMap RECURSIVE_TYPES = it.unimi.dsi.fastutil.objects.Object2IntMaps.synchronize(new Object2IntOpenHashMap<>()); // Paper ++ private final Map> TYPE_TEMPLATES = Maps.newConcurrentMap(); // Paper + private final Map> TYPES; + private final int versionKey; + private final String name; +@@ -37,17 +37,19 @@ public class Schema { + } + + protected Map> buildTypes() { +- final Map> types = Maps.newHashMap(); ++ final Map> types = Maps.newConcurrentMap(); // Paper + + final List templates = Lists.newArrayList(); + ++ synchronized (RECURSIVE_TYPES) { // Paper + for (final Object2IntMap.Entry entry : RECURSIVE_TYPES.object2IntEntrySet()) { + templates.add(DSL.check(entry.getKey(), entry.getIntValue(), getTemplate(entry.getKey()))); +- } ++ } } // Paper + + final TypeTemplate choice = templates.stream().reduce(DSL::or).get(); + final TypeFamily family = new RecursiveTypeFamily(name, choice); + ++ synchronized (TYPE_TEMPLATES) { // Paper + for (final String name : TYPE_TEMPLATES.keySet()) { + final Type type; + if (RECURSIVE_TYPES.containsKey(name)) { +@@ -56,7 +58,7 @@ public class Schema { + type = getTemplate(name).apply(family).apply(-1); + } + types.put(name, type); +- } ++ } } // Paper + return types; + } + +@@ -89,9 +91,10 @@ public class Schema { + } + + public TypeTemplate id(final String name) { ++ synchronized (RECURSIVE_TYPES) { // Paper + if (RECURSIVE_TYPES.containsKey(name)) { +- return DSL.id(RECURSIVE_TYPES.get(name)); +- } ++ return DSL.id(RECURSIVE_TYPES.getInt(name)); // Paper ++ } } // Paper + return getTemplate(name); + } + +@@ -138,9 +141,10 @@ public class Schema { + public void registerType(final boolean recursive, final DSL.TypeReference type, final Supplier template) { + TYPE_TEMPLATES.put(type.typeName(), template); + // TODO: calculate recursiveness instead of hardcoding ++ synchronized (RECURSIVE_TYPES) { // Paper + if (recursive && !RECURSIVE_TYPES.containsKey(type.typeName())) { + RECURSIVE_TYPES.put(type.typeName(), RECURSIVE_TYPES.size()); +- } ++ } } // Paper + } + + public int getVersionKey() { +diff --git a/src/main/java/com/mojang/datafixers/types/families/RecursiveTypeFamily.java b/src/main/java/com/mojang/datafixers/types/families/RecursiveTypeFamily.java +index 4a1f906837..93c2f565fd 100644 +--- a/src/main/java/com/mojang/datafixers/types/families/RecursiveTypeFamily.java ++++ b/src/main/java/com/mojang/datafixers/types/families/RecursiveTypeFamily.java +@@ -32,7 +32,7 @@ public final class RecursiveTypeFamily implements TypeFamily { + private final TypeTemplate template; + private final int size; + +- private final Int2ObjectMap> types = new Int2ObjectOpenHashMap<>(); ++ private final Int2ObjectMap> types = it.unimi.dsi.fastutil.ints.Int2ObjectMaps.synchronize(new Int2ObjectOpenHashMap<>()); // Paper + private final int hashCode; + + public RecursiveTypeFamily(final String name, final TypeTemplate template) { +-- +2.18.0 + diff --git a/scripts/importmcdev.sh b/scripts/importmcdev.sh index 2a806907c6..b509f268ce 100755 --- a/scripts/importmcdev.sh +++ b/scripts/importmcdev.sh @@ -84,7 +84,11 @@ for f in $files; do done # Import Libraries - these must always be mapped manually, no automatic stuff - +# group lib prefix files +importLibrary com.mojang datafixerupper com/mojang/datafixers \ + schemas/Schema.java \ + DataFixerUpper.java \ + types/families/RecursiveTypeFamily.java # Temporarily add new NMS dev imports here before you run paper patch