OpenSource/PaperMC
OpenSource/PaperMC
1
0
Fork 0
mirror of https://github.com/PaperMC/Paper.git synced 2025-01-03 17:52:28 +01:00
Code Issues Projects Releases Packages Wiki Activity

#1264: Load Bukkit class before creating Registry item

Browse source 
This fixes a registry load order issue, which could cause that two different instances of the same registry item could exist, when the Bukkit class was not loaded before an item was queried.

By: DerFrZocker <derrieple@gmail.com>
This commit is contained in:
CraftBukkit/Spigot 2023-10-03 07:15:37 +11:00
parent 0c3b8eb4b9
commit b3ac8604eb
2 changed files with 176 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

38
paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java
View file

@ -46,32 +46,35 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
public static <B extends Keyed> Registry<?> createRegistry(Class<B> bukkitClass, IRegistryCustom registryHolder) {
if (bukkitClass == GameEvent.class) {
return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.GAME_EVENT), CraftGameEvent::new);
return new CraftRegistry<>(GameEvent.class, registryHolder.registryOrThrow(Registries.GAME_EVENT), CraftGameEvent::new);
}
if (bukkitClass == MusicInstrument.class) {
return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.INSTRUMENT), CraftMusicInstrument::new);
return new CraftRegistry<>(MusicInstrument.class, registryHolder.registryOrThrow(Registries.INSTRUMENT), CraftMusicInstrument::new);
}
if (bukkitClass == Structure.class) {
return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.STRUCTURE), CraftStructure::new);
return new CraftRegistry<>(Structure.class, registryHolder.registryOrThrow(Registries.STRUCTURE), CraftStructure::new);
}
if (bukkitClass == StructureType.class) {
return new CraftRegistry<>(BuiltInRegistries.STRUCTURE_TYPE, CraftStructureType::new);
return new CraftRegistry<>(StructureType.class, BuiltInRegistries.STRUCTURE_TYPE, CraftStructureType::new);
}
if (bukkitClass == TrimMaterial.class) {
return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.TRIM_MATERIAL), CraftTrimMaterial::new);
return new CraftRegistry<>(TrimMaterial.class, registryHolder.registryOrThrow(Registries.TRIM_MATERIAL), CraftTrimMaterial::new);
}
if (bukkitClass == TrimPattern.class) {
return new CraftRegistry<>(registryHolder.registryOrThrow(Registries.TRIM_PATTERN), CraftTrimPattern::new);
return new CraftRegistry<>(TrimPattern.class, registryHolder.registryOrThrow(Registries.TRIM_PATTERN), CraftTrimPattern::new);
}
return null;
}
private final Class<B> bukkitClass;
private final Map<NamespacedKey, B> cache = new HashMap<>();
private final IRegistry<M> minecraftRegistry;
private final BiFunction<NamespacedKey, M, B> minecraftToBukkit;
private boolean init;
public CraftRegistry(IRegistry<M> minecraftRegistry, BiFunction<NamespacedKey, M, B> minecraftToBukkit) {
public CraftRegistry(Class<B> bukkitClass, IRegistry<M> minecraftRegistry, BiFunction<NamespacedKey, M, B> minecraftToBukkit) {
this.bukkitClass = bukkitClass;
this.minecraftRegistry = minecraftRegistry;
this.minecraftToBukkit = minecraftToBukkit;
}
@ -83,6 +86,27 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
return cached;
}
// Make sure that the bukkit class is loaded before creating an instance.
// This ensures that only one instance with a given key is created.
//
// Without this code (when bukkit class is not loaded):
// Registry#get -> #createBukkit -> (load class -> create default) -> put in cache
// Result: Registry#get != <bukkitClass>.<field> for possible one registry item
//
// With this code (when bukkit class is not loaded):
// Registry#get -> (load class -> create default) -> Registry#get -> get from cache
// Result: Registry#get == <bukkitClass>.<field>
if (!init) {
init = true;
try {
Class.forName(bukkitClass.getName());
} catch (ClassNotFoundException e) {
throw new RuntimeException("Could not load registry class " + bukkitClass, e);
}
return get(namespacedKey);
}
B bukkit = createBukkit(namespacedKey, minecraftRegistry.getOptional(CraftNamespacedKey.toMinecraft(namespacedKey)).orElse(null));
if (bukkit == null) {
return null;

145
paper-server/src/test/java/org/bukkit/RegistryLoadOrderTest.java Normal file
View file

@ -0,0 +1,145 @@
package org.bukkit;
import static org.junit.jupiter.api.Assertions.*;
import com.mojang.serialization.Lifecycle;
import java.util.function.BiFunction;
import java.util.function.Supplier;
import java.util.stream.Stream;
import net.minecraft.core.IRegistry;
import net.minecraft.core.RegistryMaterials;
import net.minecraft.resources.MinecraftKey;
import net.minecraft.resources.ResourceKey;
import org.bukkit.craftbukkit.CraftRegistry;
import org.bukkit.support.AbstractTestingBase;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
public class RegistryLoadOrderTest extends AbstractTestingBase {
private static boolean initInterface = false;
private static boolean initAbstract = false;
private static Registry<Keyed> registry;
public static Stream<Arguments> data() {
return Stream.of(
Arguments.of(
(Supplier<Boolean>) () -> initInterface,
BukkitInterfaceTestType.class,
(BiFunction<NamespacedKey, MinecraftTestType, Keyed>) CraftBukkitInterfaceTestType::new,
(Supplier<Keyed>) () -> BukkitInterfaceTestType.TEST_ONE,
(Supplier<Keyed>) () -> BukkitInterfaceTestType.TEST_TWO
),
Arguments.of(
(Supplier<Boolean>) () -> initAbstract,
BukkitAbstractTestType.class,
(BiFunction<NamespacedKey, MinecraftTestType, Keyed>) CraftBukkitAbstractTestType::new,
(Supplier<Keyed>) () -> BukkitAbstractTestType.TEST_ONE,
(Supplier<Keyed>) () -> BukkitAbstractTestType.TEST_TWO
)
);
}
@ParameterizedTest
@MethodSource("data")
public void testRegistryLoadOrder(Supplier<Boolean> init, Class<Keyed> keyedClass, BiFunction<NamespacedKey, MinecraftTestType, Keyed> minecraftToBukkit, Supplier<Keyed> first, Supplier<Keyed> second) {
testClassNotLoaded(init.get());
ResourceKey<IRegistry<MinecraftTestType>> resourceKey = ResourceKey.createRegistryKey(MinecraftKey.tryBuild("bukkit", "test-registry"));
RegistryMaterials<MinecraftTestType> minecraftRegistry = new RegistryMaterials<>(resourceKey, Lifecycle.experimental());
minecraftRegistry.register(ResourceKey.create(resourceKey, MinecraftKey.tryBuild("bukkit", "test-one")), new MinecraftTestType(), Lifecycle.experimental());
minecraftRegistry.register(ResourceKey.create(resourceKey, MinecraftKey.tryBuild("bukkit", "test-two")), new MinecraftTestType(), Lifecycle.experimental());
minecraftRegistry.freeze();
registry = new CraftRegistry<>(keyedClass, minecraftRegistry, minecraftToBukkit);
testClassNotLoaded(init.get());
Object testOne = registry.get(new NamespacedKey("bukkit", "test-one"));
Object otherTestOne = registry.get(new NamespacedKey("bukkit", "test-one"));
Object testTwo = registry.get(new NamespacedKey("bukkit", "test-two"));
assertNotNull(testOne);
assertNotNull(otherTestOne);
assertNotNull(testTwo);
assertNotNull(first.get());
assertNotNull(second.get());
assertSame(testOne, otherTestOne);
assertSame(testOne, first.get());
assertSame(otherTestOne, first.get());
assertSame(testTwo, second.get());
assertTrue(init.get());
}
private void testClassNotLoaded(boolean init) {
assertFalse(init, """
TestType class is already loaded, this test however tests the behavior when the class is not loaded.
This should normally not happen with how classes should be loaded.
Something has changed how classes are loaded and a more manual deeper look is required.
""");
}
public interface BukkitInterfaceTestType extends Keyed {
BukkitInterfaceTestType TEST_ONE = get("test-one");
BukkitInterfaceTestType TEST_TWO = get("test-two");
private static BukkitInterfaceTestType get(String key) {
initInterface = true;
if (registry == null) {
return null;
}
return (BukkitInterfaceTestType) registry.get(new NamespacedKey("bukkit", key));
}
}
public static class CraftBukkitInterfaceTestType implements BukkitInterfaceTestType {
private final NamespacedKey key;
public CraftBukkitInterfaceTestType(NamespacedKey key, MinecraftTestType minecraftTestType) {
this.key = key;
}
@NotNull
@Override
public NamespacedKey getKey() {
return key;
}
}
public abstract static class BukkitAbstractTestType implements Keyed {
public static final BukkitAbstractTestType TEST_ONE = get("test-one");
public static final BukkitAbstractTestType TEST_TWO = get("test-two");
private static BukkitAbstractTestType get(String key) {
initAbstract = true;
if (registry == null) {
return null;
}
return (BukkitAbstractTestType) registry.get(new NamespacedKey("bukkit", key));
}
}
public static class CraftBukkitAbstractTestType extends BukkitAbstractTestType {
private final NamespacedKey key;
public CraftBukkitAbstractTestType(NamespacedKey key, MinecraftTestType minecraftTestType) {
this.key = key;
}
@NotNull
@Override
public NamespacedKey getKey() {
return key;
}
}
public static class MinecraftTestType {
}
}