Kaynağa Gözat

Fix crashes due to concurrent modification of vanilla's multipart bitSet caches, closes #24

malte0811 3 yıl önce
ebeveyn
işleme
0e84664a94

+ 47 - 0
common/src/main/java/malte0811/ferritecore/mixin/dedupmultipart/MixinMultipartModel.java

@@ -0,0 +1,47 @@
+package malte0811.ferritecore.mixin.dedupmultipart;
+
+import net.minecraft.block.BlockState;
+import net.minecraft.client.renderer.model.MultipartBakedModel;
+import org.spongepowered.asm.mixin.Final;
+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.Redirect;
+
+import java.util.BitSet;
+import java.util.Map;
+
+/**
+ * The map implementation used for {@link MultipartBakedModel#bitSetCache} ({@link it.unimi.dsi.fastutil.objects.Object2ObjectOpenCustomHashMap})
+ * is not thread-safe, but getQuads is called in parallel in vanilla (and even more so in Forge with
+ * alwaysSetupTerrainOffThread=true). The only reason this works for vanilla is that the cache will never contain more
+ * than a single blockstate, since a new instance is created for each blockstate (this is probably unintentional, a map
+ * would be a weird choice for this scenario). {@link MixinMultipartBuilder} re-uses the equivalent models, so the cache
+ * can grow beyond a single element (as is probably intended). If a put-call causes the backing array to be resized
+ * concurrent get-calls can (and will) crash, so we need to synchronize them.<br>
+ * It is not clear if this implementation (naive synchronization on the cache) is optimal w.r.t.
+ * runtime/parallelization, in my experience this part of the code is not runtime-critical enough to put significant
+ * effort into fancy parallelization solutions (may change in the future).
+ */
+// Work around Java/Mixin limitations
+@SuppressWarnings("SynchronizeOnNonFinalField")
+@Mixin(MultipartBakedModel.class)
+public class MixinMultipartModel {
+    @Shadow
+    @Final
+    private Map<BlockState, BitSet> bitSetCache;
+
+    @Redirect(method = "getQuads", at = @At(value = "INVOKE", target = "Ljava/util/Map;get(Ljava/lang/Object;)Ljava/lang/Object;"))
+    public <K, V> V redirectCacheGet(Map<K, V> map, K key) {
+        synchronized (bitSetCache) {
+            return map.get(key);
+        }
+    }
+
+    @Redirect(method = "getQuads", at = @At(value = "INVOKE", target = "Ljava/util/Map;put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"))
+    public <K, V> V redirectCachePut(Map<K, V> map, K key, V value) {
+        synchronized (bitSetCache) {
+            return map.put(key, value);
+        }
+    }
+}

+ 2 - 1
common/src/main/resources/ferritecore.dedupmultipart.mixin.json

@@ -8,7 +8,8 @@
   "minVersion": "0.8",
   "plugin": "malte0811.ferritecore.mixin.dedupmultipart.Config",
   "client": [
-    "MixinMultipartBuilder"
+    "MixinMultipartBuilder",
+    "MixinMultipartModel"
   ],
   "mixins": []
 }