Fix a Forge startup race on Minecraft 1.17.1

.github/workflows/ci.yml

@@ -153,9 +153,9 @@ jobs:
           ./gradlew "${gradle_args[@]}"
           first="$RUNNER_TEMP/basemetals-first"
           mkdir -p "$first"
-          cp build/libs/BaseMetals-3.1.0.117011.jar "$first/"
-          cp build/libs/BaseMetals-3.1.0.117011-sources.jar "$first/"
-          cp build/libs/BaseMetals-3.1.0.117011-javadoc.jar "$first/"
+          cp build/libs/BaseMetals-3.1.1.117011.jar "$first/"
+          cp build/libs/BaseMetals-3.1.1.117011-sources.jar "$first/"
+          cp build/libs/BaseMetals-3.1.1.117011-javadoc.jar "$first/"
           cp build/release/SHA256SUMS "$first/"
 
       - name: Rebuild and prove byte-identical archives
@@ -171,9 +171,9 @@ jobs:
             -Dorg.gradle.java.installations.auto-download=false
           )
           ./gradlew "${gradle_args[@]}"
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.117011.jar" build/libs/BaseMetals-3.1.0.117011.jar
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.117011-sources.jar" build/libs/BaseMetals-3.1.0.117011-sources.jar
-          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.0.117011-javadoc.jar" build/libs/BaseMetals-3.1.0.117011-javadoc.jar
+          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.1.117011.jar" build/libs/BaseMetals-3.1.1.117011.jar
+          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.1.117011-sources.jar" build/libs/BaseMetals-3.1.1.117011-sources.jar
+          cmp "$RUNNER_TEMP/basemetals-first/BaseMetals-3.1.1.117011-javadoc.jar" build/libs/BaseMetals-3.1.1.117011-javadoc.jar
           cmp "$RUNNER_TEMP/basemetals-first/SHA256SUMS" build/release/SHA256SUMS
 
       - name: Run exact-OreSpawn dedicated-server probe
@@ -197,9 +197,9 @@ jobs:
           if-no-files-found: error
           retention-days: 30
           path: |
-            build/libs/BaseMetals-3.1.0.117011.jar
-            build/libs/BaseMetals-3.1.0.117011-sources.jar
-            build/libs/BaseMetals-3.1.0.117011-javadoc.jar
+            build/libs/BaseMetals-3.1.1.117011.jar
+            build/libs/BaseMetals-3.1.1.117011-sources.jar
+            build/libs/BaseMetals-3.1.1.117011-javadoc.jar
             build/release/SHA256SUMS
             CHANGELOG.txt
 

CHANGELOG.txt

@@ -1,3 +1,8 @@
+Base Metals 3.1.1.117011
+
+- Fix a startup hang when Base Metals and Forge initialize the login handshake at the same time.
+- Register the world-upgrade hook after parallel mod loading so it cannot race another mod.
+
 Base Metals 3.1.0.117011
 
 - Bring Base Metals to Minecraft 1.17.1 and Forge 37.1.1, with Java 16 for the game.

README.md

@@ -12,7 +12,7 @@ shields, bows, crossbows, arrows, and bolts. The mod also includes Crack
 Hammers, scythes, metal anvils, the human detector, villager trades,
 advancements, and extra loot to find in chests.
 
-This is Base Metals `3.1.0.117011` for Minecraft 1.17.1. It brings the 1.16.5
+This is Base Metals `3.1.1.117011` for Minecraft 1.17.1. It brings the 1.16.5
 release forward and adds raw metals, while keeping existing blocks and items.
 MMDLib and Additional Loot Tables are no longer needed. OreSpawn places the
 ores, and Base Metals supplies the metals and their recipes.
@@ -35,7 +35,7 @@ without changing their collision or placement.
 - Java 16
 - OreSpawn `[4.1.0.117011,5.0.0)` on both client and server
 
-Put `BaseMetals-3.1.0.117011.jar` and OreSpawn `4.1.0.117011` (or a later
+Put `BaseMetals-3.1.1.117011.jar` and OreSpawn `4.1.0.117011` (or a later
 compatible OreSpawn 4 build for 1.17.1) in the `mods` directory. MMDLib and
 Additional Loot Tables are not dependencies. Mineralogy is optional.
 
@@ -220,7 +220,7 @@ and passes its temporary repository through `orespawnVerificationRepository`.
 Normal development does not need a local OreSpawn checkout or repository.
 
 Release artifacts use Maven coordinate
-`zone.moddev.mc.basemetals:BaseMetals:3.1.0.117011`.
+`zone.moddev.mc.basemetals:BaseMetals:3.1.1.117011`.
 
 Report defects through the
 [Base Metals issue tracker](https://github.com/MinecraftModDevelopmentMods/BaseMetals/issues).

docs/MIGRATION.md

@@ -6,7 +6,7 @@ blocks cannot be reconstructed by Mojang's vanilla fixer alone, so Base Metals
 installs a narrowly scoped migration hook before Forge reads an old world.
 
 Always upgrade a copy and retain the original 1.10/1.12 world. Install Base
-Metals `3.1.0.117011`, OreSpawn `4.1.0.117011`, and any other 1.17.1 mods needed
+Metals `3.1.1.117011`, OreSpawn `4.1.0.117011`, and any other 1.17.1 mods needed
 by that world. Do not carry MMDLib, Additional Loot Tables, the OreSpawn 3
 plugin, or Base Metals' old fallback generator into the new instance.
 

docs/VERSIONS.md

@@ -1,6 +1,6 @@
 # Supported versions
 
-Base Metals `3.1.0.117011` targets Minecraft `1.17.1`, Forge `37.1.1`, and
+Base Metals `3.1.1.117011` targets Minecraft `1.17.1`, Forge `37.1.1`, and
 Java 16. The Gradle build itself runs on Java `17.0.1+12` and uses ForgeGradle
 `7.0.34` with Gradle `9.6.1`; those build-time runtimes do not change the Java
 16 requirement of the released mod. Release builds use Temurin `16.0.2+7`.
@@ -19,4 +19,4 @@ file before staging it in a temporary Maven repository.
 The schema-3 provider supports the OreSpawn versions in the runtime range above.
 
 Release artifacts use Maven coordinate
-`zone.moddev.mc.basemetals:BaseMetals:3.1.0.117011`.
+`zone.moddev.mc.basemetals:BaseMetals:3.1.1.117011`.

gradle.properties

@@ -40,7 +40,7 @@ mcp_version=20210706.113038
 mod_id=basemetals
 mod_name=Base Metals
 mod_license=LGPL-2.1
-mod_version=3.1.0.117011
+mod_version=3.1.1.117011
 
 # Maven coordinates and credits.
 mod_group=zone.moddev.mc.basemetals

gradle/release/artifacts.gradle

@@ -131,7 +131,7 @@ tasks.register('verifyReleaseConfiguration') {
                 loaderCode: project.loader_code.toString(), javaVersion: project.java_version.toString(),
                 gradleJavaVersion: project.gradle_java_version.toString(),
                 curseForgeProjectId: project.curseforge_project_id.toString()]
-        def expected = [modVersion: '3.1.0.117011', modGroup: expectedMavenGroup,
+        def expected = [modVersion: '3.1.1.117011', modGroup: expectedMavenGroup,
                 minecraftVersion: '1.17.1', forgeVersion: '37.1.1',
                 forgeInstallerSha256: '31F2319B1B9CA491AACCC020887240F8524A4FF2A8E0F8B795E17E889CBC043B',
                 mappingChannel: 'official', mappingVersion: '1.17.1',
@@ -258,7 +258,7 @@ tasks.register('verifyReleaseArtifacts') {
                 throw new GradleException('Release manifest contains the wrong identity')
             }
             String modsToml = zip.getInputStream(zip.getEntry('META-INF/mods.toml')).getText('UTF-8')
-            if (modsToml.contains('${') || !modsToml.contains('version="3.1.0.117011"')
+            if (modsToml.contains('${') || !modsToml.contains('version="3.1.1.117011"')
                     || !modsToml.contains('versionRange="[4.1.0.117011,5.0.0)"')
                     || !modsToml.contains('loaderVersion="[37,38)"')
                     || !modsToml.contains('versionRange="[37,38)"')

src/clientIntegrationTest/resources/META-INF/mods.toml

@@ -11,7 +11,7 @@ description='''Build-only packaged-client checks for Base Metals 1.17.1.'''
 [[dependencies.basemetalsclientprobe]]
 modId="basemetals"
 mandatory=true
-versionRange="[3.1.0.117011]"
+versionRange="[3.1.1.117011]"
 ordering="AFTER"
 side="CLIENT"
 

src/integrationTest/resources/META-INF/mods.toml

@@ -11,7 +11,7 @@ description='''Build-only lifecycle checks for Base Metals 1.17.1.'''
 [[dependencies.basemetalsprobe]]
 modId="basemetals"
 mandatory=true
-versionRange="[3.1.0.117011]"
+versionRange="[3.1.1.117011]"
 ordering="AFTER"
 side="BOTH"
 

src/main/java/zone/moddev/mc/basemetals/BaseMetals.java

@@ -18,6 +18,7 @@
 import net.minecraftforge.fml.ModLoadingContext;
 import net.minecraftforge.fml.common.Mod;
 import net.minecraftforge.fml.config.ModConfig;
+import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent;
 import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
 
 @Mod(BaseMetals.MOD_ID)
@@ -27,14 +28,20 @@ public final class BaseMetals {
 
     public BaseMetals() {
         ModTags.initialize();
-        LegacyWorldDataHook.register();
-        ContentModeNetwork.register();
         ModContent.initializeFluids();
         ContentModeLootCondition.register();
         ModEntities.initialize();
         ModLoadingContext.get().registerConfig(ModConfig.Type.COMMON, BaseMetalsConfig.SPEC);
         FMLJavaModLoadingContext.get().getModEventBus().addListener(BaseMetalsConfig::onConfigLoading);
+        FMLJavaModLoadingContext.get().getModEventBus().addListener(this::commonSetup);
         MinecraftForge.EVENT_BUS.register(new BaseMetalsEvents());
         DistExecutor.runWhenOn(Dist.CLIENT, () -> ClientSetup::register);
     }
+
+    private void commonSetup(FMLCommonSetupEvent event) {
+        // Register after parallel mod loading, when Forge's handshake classes
+        // and the shared world-hook list can be accessed on one thread.
+        event.enqueueWork(ContentModeNetwork::register);
+        event.enqueueWork(LegacyWorldDataHook::register);
+    }
 }

src/main/resources/META-INF/mods.toml

@@ -5,7 +5,7 @@ issueTrackerURL="https://github.com/MinecraftModDevelopmentMods/BaseMetals/issue
 
 [[mods]]
 modId="basemetals"
-version="3.1.0.117011"
+version="3.1.1.117011"
 displayName="Base Metals"
 displayURL="https://www.curseforge.com/minecraft/mc-mods/base-metals"
 authors="Cyanobacterium, jriwanek, dshadowwolf, GetDizzy611, SkyBlade1978, the MMD Team"

src/test/java/zone/moddev/mc/basemetals/SerializedSetupTest.java

@@ -0,0 +1,68 @@
+package zone.moddev.mc.basemetals;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.InputStream;
+import java.util.HashSet;
+import java.util.Set;
+import org.junit.jupiter.api.Test;
+import org.objectweb.asm.ClassReader;
+import org.objectweb.asm.Handle;
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.ClassNode;
+import org.objectweb.asm.tree.InvokeDynamicInsnNode;
+import org.objectweb.asm.tree.MethodInsnNode;
+import org.objectweb.asm.tree.MethodNode;
+
+class SerializedSetupTest {
+    private static final String MOD = "zone/moddev/mc/basemetals/";
+    private static final String SETUP_EVENT = "net/minecraftforge/fml/event/lifecycle/FMLCommonSetupEvent";
+    private static final Set<String> REGISTRATIONS = Set.of(
+            MOD + "migration/LegacyWorldDataHook", MOD + "network/ContentModeNetwork");
+
+    @Test
+    void registersSharedForgeStateOnlyThroughCommonSetupQueue() throws Exception {
+        ClassNode entry = new ClassNode();
+        try (InputStream input = getClass().getResourceAsStream("/" + MOD + "BaseMetals.class")) {
+            assertNotNull(input);
+            new ClassReader(input).accept(entry, 0);
+        }
+        Set<String> queued = new HashSet<>();
+        int subscribed = 0;
+        for (MethodNode method : entry.methods) {
+            for (AbstractInsnNode instruction : method.instructions.toArray()) {
+                if (instruction instanceof MethodInsnNode call) {
+                    assertFalse(REGISTRATIONS.contains(call.owner) && call.name.equals("register"),
+                            "Direct registration can race Forge during " + method.name);
+                }
+                if (!(instruction instanceof InvokeDynamicInsnNode callback)) continue;
+                AbstractInsnNode next = instruction.getNext();
+                while (next != null && next.getOpcode() < 0) next = next.getNext();
+                for (Object argument : callback.bsmArgs) {
+                    if (!(argument instanceof Handle target)) continue;
+                    if (target.getOwner().equals(entry.name) && target.getName().equals("commonSetup")) {
+                        assertEquals("<init>", method.name);
+                        assertTrue(next instanceof MethodInsnNode listener
+                                && listener.owner.equals("net/minecraftforge/eventbus/api/IEventBus")
+                                && listener.name.equals("addListener"));
+                        subscribed++;
+                    }
+                    if (REGISTRATIONS.contains(target.getOwner()) && target.getName().equals("register")) {
+                        assertEquals("commonSetup", method.name);
+                        assertEquals("(L" + SETUP_EVENT + ";)V", method.desc);
+                        assertTrue(next instanceof MethodInsnNode enqueue
+                                && enqueue.owner.equals(SETUP_EVENT)
+                                && enqueue.name.equals("enqueueWork")
+                                && enqueue.desc.startsWith("(Ljava/lang/Runnable;)"));
+                        assertTrue(queued.add(target.getOwner()), "Queue each registration once");
+                    }
+                }
+            }
+        }
+        assertEquals(1, subscribed);
+        assertEquals(REGISTRATIONS, queued);
+    }
+}