Conversation
The per-dimension LostCityTerrainFeature holds a single shared ChunkDriver, and IDimensionInfo.setWorld() swaps the shared region reference right before each generation, with no synchronization at the feature entry points. The vanilla feature step can run concurrently for different chunks on worker threads, so one thread's driver teardown (setPrimer restore) or region swap can hit in the middle of another thread's generation. Observed crashes: - NullPointerException: "this.primer" is null at ChunkDriver.current (via Spheres.fillSphere / LostCitySphereFeature.place) - NullPointerException: "this.region" is null at ChunkDriver.getBlock - IllegalStateException: Requested chunk unavailable during world generation at WorldGenRegion.getChunk via getBlockSafe -> updateAdjacent -> correct - Generation running against another thread's WorldGenRegion, visible as reads/writes 3-8 chunks outside the current region This reproduces near-instantly with parallel worldgen mods like C2ME but the race also exists under vanilla parallel chunk generation. Fixes: - LostCityFeature.place and LostCitySphereFeature.place now synchronize the setWorld + generate sequence on the shared per-dimension terrain feature instance (both features use the same monitor), so Lost Cities' own generation is serialized per dimension while the rest of worldgen stays parallel - ChunkDriver.getBlockSafe/updateAdjacent treat neighbouring chunks that are not available in the current WorldGenRegion as air / skip the update instead of crashing; these paths only drive cosmetic connection states (fences, walls, stairs) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
|
I'm actually working on a big reword of Lost Cities in 1.20.1 branch which I will later port over to 1.21. These changes include work around thread-safety and such so I would wait a bit with this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Each dimension caches a single
LostCityTerrainFeature, which owns a single sharedChunkDriver, andIDimensionInfo.setWorld()swaps the shared region reference right before every generation. The feature step of chunk generation can run concurrently for different chunks on worker threads, so there is a race: one thread's driver teardown (thesetPrimer(oldRegion, oldChunk)restore at the end ofgenerate()) or region swap can land in the middle of another thread's generation.Crashes we captured during world creation ("Preparing spawn area"):
java.lang.NullPointerException: Cannot invoke "ChunkAccess.getPos()" because "this.primer" is nullatChunkDriver.current(viaSpheres.fillSphere←LostCitySphereFeature.place)NullPointerException: "this.region" is nullatChunkDriver.getBlockIllegalStateException: Requested chunk unavailable during world generationatWorldGenRegion.getChunkviagetBlockSafe→updateAdjacent→correct→blockWorldGenRegion(visible as reads/writes 3–8 chunks outside the current region)This reproduces near-instantly with parallel-worldgen mods (C2ME on the Fabric side is how we hit it), but the race itself exists under vanilla parallel chunk generation too.
Fix
LostCityFeature.placeandLostCitySphereFeature.placenow synchronize thesetWorld+ generate sequence on the shared per-dimension terrain feature instance (both features use the same monitor). Only Lost Cities' own generation is serialized per dimension; the rest of worldgen stays parallel.ChunkDriver.getBlockSafe/updateAdjacenttreat neighbouring chunks that aren't available in the currentWorldGenRegionas air / skip the update instead of throwing — these reads only drive cosmetic connection states (fences, walls, stairs), so a neighbour read can never kill chunk generation anymore.Verification
./gradlew compileJavapasses on this branch (1.21.11_neo).defaultandbiosphereprofiles: no exceptions, cities and sphere shells generate. I have not run the NeoForge build in-game.🤖 Generated with Claude Code