Fix Fabric-side thread safety for Tags obtained from TagRegistry

This commit is contained in:
Player 2019-11-03 18:16:29 +01:00
parent 7a0d585458
commit 6d8d2cc9dc
2 changed files with 46 additions and 25 deletions

View file

@ -41,19 +41,7 @@ public final class TagRegistry {
private TagRegistry() { } private TagRegistry() { }
public static <T> Tag<T> create(Identifier id, Supplier<TagContainer<T>> containerSupplier) { public static <T> Tag<T> create(Identifier id, Supplier<TagContainer<T>> containerSupplier) {
return new TagDelegate<T>(id, null) { return new TagDelegate<>(id, containerSupplier);
private TagContainer<T> container;
@Override
protected void onAccess() {
TagContainer<T> currContainer = containerSupplier.get();
if (container != currContainer) {
container = currContainer;
delegate = container.getOrCreate(this.getId());
}
}
};
} }
public static Tag<Block> block(Identifier id) { public static Tag<Block> block(Identifier id) {

View file

@ -17,35 +17,68 @@
package net.fabricmc.fabric.impl.tag.extension; package net.fabricmc.fabric.impl.tag.extension;
import java.util.Collection; import java.util.Collection;
import java.util.function.Supplier;
import net.minecraft.tag.Tag; import net.minecraft.tag.Tag;
import net.minecraft.tag.TagContainer;
import net.minecraft.util.Identifier; import net.minecraft.util.Identifier;
public class TagDelegate<T> extends Tag<T> { public final class TagDelegate<T> extends Tag<T> {
protected Tag<T> delegate; private final Supplier<TagContainer<T>> containerSupplier;
private volatile Target<T> target;
public TagDelegate(Identifier id, Tag<T> delegate) { public TagDelegate(Identifier id, Supplier<TagContainer<T>> containerSupplier) {
super(id); super(id);
this.delegate = delegate;
}
protected void onAccess() { } this.containerSupplier = containerSupplier;
}
@Override @Override
public boolean contains(T var1) { public boolean contains(T var1) {
onAccess(); return getTag().contains(var1);
return delegate.contains(var1);
} }
@Override @Override
public Collection<T> values() { public Collection<T> values() {
onAccess(); return getTag().values();
return delegate.values();
} }
@Override @Override
public Collection<Tag.Entry<T>> entries() { public Collection<Tag.Entry<T>> entries() {
onAccess(); return getTag().entries();
return delegate.entries(); }
/**
* Retrieve the tag this delegate is pointing to, computing it if missing or outdated.
*
* <p>Thread safety is being ensured by using an immutable holder object for consistently retrieving both result
* and condition, volatile for safe publishing and assuming TagContainer.getOrCreate is safe to call concurrently.
*
* <p>It should be possible to exploit a benign data race on this.target by removing volatile, but this option
* hasn't been chosen yet since a performance problem in the area is yet to be proven.
*/
private Tag<T> getTag() {
Target<T> target = this.target;
TagContainer<T> reqContainer = containerSupplier.get();
Tag<T> ret;
if (target == null || target.container != reqContainer) {
ret = reqContainer.getOrCreate(getId());
this.target = new Target<>(reqContainer, ret);
} else {
ret = target.tag;
}
return ret;
}
private static final class Target<T> {
Target(TagContainer<T> container, Tag<T> tag) {
this.container = container;
this.tag = tag;
}
final TagContainer<T> container;
final Tag<T> tag;
} }
} }