diff --git a/common/src/main/java/com/player2/playerengine/mixins/MixinEventFactoryEventImpl.java b/common/src/main/java/com/player2/playerengine/mixins/MixinEventFactoryEventImpl.java new file mode 100644 index 00000000..c4f5ed6f --- /dev/null +++ b/common/src/main/java/com/player2/playerengine/mixins/MixinEventFactoryEventImpl.java @@ -0,0 +1,63 @@ +package com.player2.playerengine.mixins; + +import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.injection.At; +import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.ListIterator; +import java.util.function.Function; + +/** + * Fixes ConcurrentModificationException in Architectury EventFactory. + * + * Architectury API issue #653: EventFactory uses a plain ArrayList for event + * listeners, which is not thread-safe. When a mod registers a listener during + * event dispatch (e.g., during the first server tick), the iterator throws + * ConcurrentModificationException. + * + * This mixin replaces the ArrayList with a SnapshotArrayList subclass that + * returns snapshot-based iterators, making concurrent registration during + * iteration safe. We must extend ArrayList because the field type is ArrayList, + * not List. + */ +@Mixin(targets = "dev.architectury.event.EventFactory$EventImpl", remap = false) +public class MixinEventFactoryEventImpl { + + @Shadow + private ArrayList listeners; + + /** + * An ArrayList subclass whose iterators operate on a snapshot of the backing + * array at the time of iterator creation. This means concurrent add/remove + * during iteration won't throw ConcurrentModificationException. + */ + public static class SnapshotArrayList extends ArrayList { + @Override + public Iterator iterator() { + // Return an iterator over a snapshot copy + return new ArrayList<>(this).iterator(); + } + + @Override + public ListIterator listIterator() { + return new ArrayList<>(this).listIterator(); + } + + @Override + public ListIterator listIterator(int index) { + return new ArrayList<>(this).listIterator(index); + } + } + + @Inject(method = "", at = @At("RETURN")) + private void playerengine$replaceListenersWithSnapshotList(Function function, CallbackInfo ci) { + // Replace the plain ArrayList with our SnapshotArrayList. + // The snapshot iterator pattern means that iteration (event dispatch) sees + // a frozen copy, while registration can safely modify the real list. + this.listeners = new SnapshotArrayList<>(); + } +} diff --git a/common/src/main/java/com/player2/playerengine/mixins/baritone/MixinItemStack.java b/common/src/main/java/com/player2/playerengine/mixins/baritone/MixinItemStack.java index 3c157765..443a1802 100644 --- a/common/src/main/java/com/player2/playerengine/mixins/baritone/MixinItemStack.java +++ b/common/src/main/java/com/player2/playerengine/mixins/baritone/MixinItemStack.java @@ -19,11 +19,39 @@ public abstract class MixinItemStack implements IItemStack { @Unique private int baritoneHash; + // Thread-local recursion guard: prevents infinite loop when getDamageValue() + // triggers ItemStack.copy -> ItemStack. -> recalculateHash -> getDamageValue + // (e.g. Silent Gear MainPartItem.getMaxDamage -> PartInstance -> ItemStack.copy) + @Unique + private static final ThreadLocal playerengine$inRecalc = ThreadLocal.withInitial(() -> Boolean.FALSE); + @Shadow public abstract int getDamageValue(); private void recalculateHash() { - this.baritoneHash = this.item == null ? -1 : this.item.hashCode() + this.getDamageValue(); + if (this.item == null) { + this.baritoneHash = -1; + return; + } + // If we're already inside recalculateHash on this thread, skip to avoid recursion. + // This breaks the cycle: recalculateHash -> getDamageValue -> getMaxDamage -> + // PartInstance -> ItemStack.copy -> ItemStack. -> recalculateHash + if (playerengine$inRecalc.get()) { + this.baritoneHash = this.item.hashCode(); + return; + } + playerengine$inRecalc.set(Boolean.TRUE); + try { + this.baritoneHash = this.item.hashCode() + this.getDamageValue(); + } catch (Throwable t) { + // Catches failure modes during ItemStack init: + // - IllegalStateException: NeoForge config not loaded (e.g. ConstructionStick) + // - RuntimeException: client-only class loaded on DEDICATED_SERVER + // - StackOverflowError: any remaining deep recursion + this.baritoneHash = this.item.hashCode(); + } finally { + playerengine$inRecalc.set(Boolean.FALSE); + } } @Inject( diff --git a/common/src/main/resources/playerengine.mixins.json b/common/src/main/resources/playerengine.mixins.json index 57ba7e0f..51d69bb5 100644 --- a/common/src/main/resources/playerengine.mixins.json +++ b/common/src/main/resources/playerengine.mixins.json @@ -8,15 +8,16 @@ "defaultRequire": 1 }, "client": [ + "ClientBlockBreakMixin", + "PlayerCollidesWithEntityMixin" ], "mixins": [ - "ClientBlockBreakMixin", "ClientConnectionAccessor", "EntityAnimationSwungMixin", "LivingEntityMixin", "MixinAbstractFurnaceBlockEntity", + "MixinEventFactoryEventImpl", "PersistentProjectileEntityAccessor", - "PlayerCollidesWithEntityMixin", "PlayerDamageMixin", "WorldBlockModifiedMixin", "baritone.MixinBucketItem", @@ -28,4 +29,4 @@ "baritone.MixinServerCommandSource", "baritone.MixinUtil" ] -} \ No newline at end of file +}