Skip to content

Commit 873e3bd

Browse files
committed
Fix race condition when mods create tags on different threads
1 parent 6b37051 commit 873e3bd

File tree

2 files changed

+79
-0
lines changed

2 files changed

+79
-0
lines changed
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package org.embeddedt.modernfix.common.mixin.bugfix.concurrency;
2+
3+
import net.minecraft.core.HolderSet;
4+
import net.minecraft.core.MappedRegistry;
5+
import net.minecraft.tags.TagKey;
6+
import org.spongepowered.asm.mixin.Mixin;
7+
import org.spongepowered.asm.mixin.Overwrite;
8+
import org.spongepowered.asm.mixin.Shadow;
9+
10+
import java.util.IdentityHashMap;
11+
import java.util.Map;
12+
13+
@Mixin(value = MappedRegistry.class, priority = 500)
14+
public abstract class MappedRegistryMixin<T> {
15+
@Shadow private volatile Map<TagKey<T>, HolderSet.Named<T>> tags;
16+
17+
@Shadow protected abstract HolderSet.Named<T> createTag(TagKey<T> key);
18+
19+
/**
20+
* @author embeddedt (issue found by Uncandango)
21+
* @reason vanilla does not use the correct double-checked locking paradigm, which leads to race conditions
22+
*/
23+
@Overwrite
24+
public HolderSet.Named<T> getOrCreateTag(TagKey<T> key) {
25+
HolderSet.Named<T> named = this.tags.get(key);
26+
if (named == null) {
27+
// synchronize and check again - this is the bugfix
28+
synchronized (this) {
29+
named = this.tags.get(key);
30+
if (named == null) {
31+
named = this.createTag(key);
32+
Map<TagKey<T>, HolderSet.Named<T>> map = new IdentityHashMap<>(this.tags);
33+
map.put(key, named);
34+
this.tags = map;
35+
}
36+
}
37+
}
38+
return named;
39+
}
40+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package org.embeddedt.modernfix.forge.mixin.bugfix.concurrency;
2+
3+
import net.minecraft.core.HolderSet;
4+
import net.minecraft.tags.TagKey;
5+
import org.spongepowered.asm.mixin.Mixin;
6+
import org.spongepowered.asm.mixin.Overwrite;
7+
import org.spongepowered.asm.mixin.Shadow;
8+
9+
import java.util.IdentityHashMap;
10+
import java.util.Map;
11+
12+
@Mixin(targets = {"net/minecraftforge/registries/NamespacedWrapper"}, priority = 500)
13+
public abstract class NamespacedWrapperMixin<T> {
14+
@Shadow private volatile Map<TagKey<T>, HolderSet.Named<T>> tags;
15+
16+
@Shadow(aliases = {"createTag"}) protected abstract HolderSet.Named<T> m_211067_(TagKey<T> key);
17+
18+
/**
19+
* @author embeddedt (issue found by Uncandango)
20+
* @reason vanilla does not use the correct double-checked locking paradigm, which leads to race conditions
21+
*/
22+
@Overwrite
23+
public HolderSet.Named<T> getOrCreateTag(TagKey<T> key) {
24+
HolderSet.Named<T> named = this.tags.get(key);
25+
if (named == null) {
26+
// synchronize and check again - this is the bugfix
27+
synchronized (this) {
28+
named = this.tags.get(key);
29+
if (named == null) {
30+
named = this.m_211067_(key);
31+
Map<TagKey<T>, HolderSet.Named<T>> map = new IdentityHashMap<>(this.tags);
32+
map.put(key, named);
33+
this.tags = map;
34+
}
35+
}
36+
}
37+
return named;
38+
}
39+
}

0 commit comments

Comments
 (0)