From c4c58dc229e97fd54d277807534346fc186df004 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..c8e7a8aa10 100644 --- a/src/main/java/com/mojang/datafixers/DataFixerUpper.java +++ b/src/main/java/com/mojang/datafixers/DataFixerUpper.java @@ -65,7 +65,7 @@ public class DataFixerUpper implements DataFixer { private final Int2ObjectSortedMap schemas; private final List globalList; private final IntSortedSet fixerVersions; - private final Long2ObjectMap rules = new Long2ObjectOpenHashMap<>(); + private final Long2ObjectMap rules = it.unimi.dsi.fastutil.longs.Long2ObjectMaps.synchronize(new Long2ObjectOpenHashMap<>()); // Paper protected DataFixerUpper(final Int2ObjectSortedMap schemas, final List globalList, final IntSortedSet fixerVersions) { this.schemas = schemas; @@ -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/functions/PointFree.java b/src/main/java/com/mojang/datafixers/functions/PointFree.java index 0d88490f77..028942b8ea 100644 --- a/src/main/java/com/mojang/datafixers/functions/PointFree.java +++ b/src/main/java/com/mojang/datafixers/functions/PointFree.java @@ -14,7 +14,7 @@ public abstract class PointFree { private Function, T> value; @SuppressWarnings("ConstantConditions") - public Function, T> evalCached() { + public synchronized Function, T> evalCached() { // Paper if (!initialized) { initialized = true; value = eval(); 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