From 873e3bd67654c0efe8a24237eeb90dea94a3de77 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sat, 12 Jul 2025 22:22:58 -0400 Subject: [PATCH] Fix race condition when mods create tags on different threads --- .../concurrency/MappedRegistryMixin.java | 40 +++++++++++++++++++ .../concurrency/NamespacedWrapperMixin.java | 39 ++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/concurrency/MappedRegistryMixin.java create mode 100644 forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/concurrency/NamespacedWrapperMixin.java diff --git a/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/concurrency/MappedRegistryMixin.java b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/concurrency/MappedRegistryMixin.java new file mode 100644 index 00000000..b8423540 --- /dev/null +++ b/common/src/main/java/org/embeddedt/modernfix/common/mixin/bugfix/concurrency/MappedRegistryMixin.java @@ -0,0 +1,40 @@ +package org.embeddedt.modernfix.common.mixin.bugfix.concurrency; + +import net.minecraft.core.HolderSet; +import net.minecraft.core.MappedRegistry; +import net.minecraft.tags.TagKey; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Overwrite; +import org.spongepowered.asm.mixin.Shadow; + +import java.util.IdentityHashMap; +import java.util.Map; + +@Mixin(value = MappedRegistry.class, priority = 500) +public abstract class MappedRegistryMixin { + @Shadow private volatile Map, HolderSet.Named> tags; + + @Shadow protected abstract HolderSet.Named createTag(TagKey key); + + /** + * @author embeddedt (issue found by Uncandango) + * @reason vanilla does not use the correct double-checked locking paradigm, which leads to race conditions + */ + @Overwrite + public HolderSet.Named getOrCreateTag(TagKey key) { + HolderSet.Named named = this.tags.get(key); + if (named == null) { + // synchronize and check again - this is the bugfix + synchronized (this) { + named = this.tags.get(key); + if (named == null) { + named = this.createTag(key); + Map, HolderSet.Named> map = new IdentityHashMap<>(this.tags); + map.put(key, named); + this.tags = map; + } + } + } + return named; + } +} diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/concurrency/NamespacedWrapperMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/concurrency/NamespacedWrapperMixin.java new file mode 100644 index 00000000..49bc8274 --- /dev/null +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/bugfix/concurrency/NamespacedWrapperMixin.java @@ -0,0 +1,39 @@ +package org.embeddedt.modernfix.forge.mixin.bugfix.concurrency; + +import net.minecraft.core.HolderSet; +import net.minecraft.tags.TagKey; +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Overwrite; +import org.spongepowered.asm.mixin.Shadow; + +import java.util.IdentityHashMap; +import java.util.Map; + +@Mixin(targets = {"net/minecraftforge/registries/NamespacedWrapper"}, priority = 500) +public abstract class NamespacedWrapperMixin { + @Shadow private volatile Map, HolderSet.Named> tags; + + @Shadow(aliases = {"createTag"}) protected abstract HolderSet.Named m_211067_(TagKey key); + + /** + * @author embeddedt (issue found by Uncandango) + * @reason vanilla does not use the correct double-checked locking paradigm, which leads to race conditions + */ + @Overwrite + public HolderSet.Named getOrCreateTag(TagKey key) { + HolderSet.Named named = this.tags.get(key); + if (named == null) { + // synchronize and check again - this is the bugfix + synchronized (this) { + named = this.tags.get(key); + if (named == null) { + named = this.m_211067_(key); + Map, HolderSet.Named> map = new IdentityHashMap<>(this.tags); + map.put(key, named); + this.tags = map; + } + } + } + return named; + } +}