From 6b37051980f8d5cb148a975c3f57183fb4e0d38e Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Fri, 4 Jul 2025 18:42:39 -0400 Subject: [PATCH 1/3] Fix faster_ingredients bypassing defensive copy of ItemValue Kudos to @nutant233 for noticing this --- .../forge/mixin/perf/faster_ingredients/IngredientMixin.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/faster_ingredients/IngredientMixin.java b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/faster_ingredients/IngredientMixin.java index e33ca3fd..2840e2f1 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/faster_ingredients/IngredientMixin.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/mixin/perf/faster_ingredients/IngredientMixin.java @@ -142,9 +142,7 @@ public abstract class IngredientMixin implements ExtendedIngredient { private ItemStack[] computeItemsArray() { // Fast path for case with one item if (this.values.length == 1) { - if (this.values[0] instanceof Ingredient.ItemValue itemValue) { - return new ItemStack[] { itemValue.item }; - } else if (this.values[0] instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) { + if (this.values[0] instanceof Ingredient.TagValue tagValue && mfix$areTagsAvailable()) { var tag = BuiltInRegistries.ITEM.getTag(tagValue.tag); if (tag.isPresent() && tag.get().size() > 0) { var holderSet = tag.get(); 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 2/3] 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; + } +} From 65ab37b819de85a7ee7021782c5951a3f6966d4a Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Sun, 13 Jul 2025 14:31:58 -0400 Subject: [PATCH 3/3] Fix reobfuscation issue --- .../forge/mixin/bugfix/concurrency/NamespacedWrapperMixin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 49bc8274..630a4715 100644 --- 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 @@ -11,7 +11,7 @@ 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 = {"tags"}) private volatile Map, HolderSet.Named> tags; @Shadow(aliases = {"createTag"}) protected abstract HolderSet.Named m_211067_(TagKey key);