Fix parallel structure generation race in NetherFortressPieces and StrongholdPieces - #192
Open
lisolaris wants to merge 1 commit into
Open
Fix parallel structure generation race in NetherFortressPieces and StrongholdPieces#192lisolaris wants to merge 1 commit into
lisolaris wants to merge 1 commit into
Conversation
This was referenced Aug 10, 2026
…ieces STRUCTURE_STARTS is generated in parallel, but the piece weight state of these classes is shared static and only safe under vanilla's serial worldgen. Full analysis and verification: see issue Tuinity#191.
lisolaris
force-pushed
the
fix/parallel-structure-generate
branch
from
August 10, 2026 11:38
7490830 to
01cba2f
Compare
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
Fixes the race reported in #191 — parallel
STRUCTURE_STARTSgeneration corrupting nether fortresses and strongholds. See the issue for reproduction and evidence.Changes
NetherFortressPieces$StartPieceMixin: theStartPiececonstructor now copies eachPieceWeightinstead of resetting and sharing the static array elements; the vanillaplaceCount = 0reset is discarded (the copies start at 0). The static arrays are never mutated after class loading. TheList.addredirect guards against future non-PieceWeightadditions (the constructor currently contains exactly two, both weight-loop calls) — non-weight elements are added unchanged.StrongholdPiecesMixin+StrongholdPieces$StairsDownMixin: the static generation state moves to a per-threadStrongholdState(patches/structure/StrongholdState.java, a plain helper class shared by both mixins).NetherFortressPieces$PieceWeight,StrongholdPieces$PieceWeight,StrongholdPieces$StrongholdPieceandSTRONGHOLD_PIECE_WEIGHTSfor the mixins to reference (compiled in via loom for Fabric; converted to an access transformer for NeoForge).Unlike
NetherFortressPieces, theStrongholdPiecesweights cannot be copied per structure: theLibrary/PortalRoomentries are anonymous subclasses whose placement constraints (depth > 4/depth > 5) would be lost by copying. So instead of per-structure copies, each thread keeps an isolatedStrongholdState, andplaceCountis tracked in anIdentityHashMapkeyed by the shared weight object; the depth constraints are re-implemented viapieceClasschecks, equivalent to the overrides.Why ThreadLocal?
The per-thread state preserves vanilla's existing execution assumption: a structure generation task executes synchronously on one worker thread without yielding. Therefore concurrent structures receive isolated state, while independent structures still generate in parallel.
ThreadLocal lifetime
The per-thread state is re-initialised at every structure generation entry. Vanilla's
StrongholdStructure.generatePiecescallsresetPieces()before constructing eachStartPiece(once per do-while retry), and the@OverwritedresetPieces()clears the thread's state — it rebuildscurrentPieces, and clearsplaceCountsandimposedPiece. So a worker thread reused across structures never carries state into the next one: the next structure's entry always resets it. The nether fortress fix needs no such handling because its weight objects are per-StartPiececopies.Semantics
Only the state access location changes — no
nextIntcall order or evaluation order is touched. Only ownership changes; algorithmic behaviour does not. Single-threaded generation is bit-for-bit identical to vanilla.Verification
MixinAuditTestpasses on both Fabric and NeoForge (all mixins apply cleanly).5535345(instrumented before/after builds on both loaders):8→10skipping 9), premature seal-off (27 pieces vs vanilla 121, castle sections never start, seal-off ratio ~4× vanilla).[18,17](generated alone) is piece-for-piece identical to vanilla; the two concurrently generated fortresses[-6,-5],[-5,7]match the vanilla tree (120/99 non-null pieces, castle sections present, seal-off ratio back to vanilla levels). Fabric and NeoForge AFTER builds match each other value-for-value (cross-platform determinism).5535345(instrumented, both loaders): 1367 events; no behavioural differences from vanilla were observed —PortalRoomexactly once per structure (tries=1), first pieceFiveCrossing(theimposedPiecechain that would break if the state were not shared correctly), and unchanged NULL /FillerCorridorseal-off paths. This confirms the fix does not change stronghold generation behaviour. Note this is a regression check only: the stronghold race itself cannot be exercised in normal play (sparse start positions) and was not reproduced, so the stronghold fix is validated by the identical shared-state pattern rather than by observing the failure.Notes
StructureStartinstantiation in the classes themselves) than the mixin approach here. Both implementations remove cross-structure shared mutable state; the difference is only due to the integration mechanism.About AI Assistance
The debug and fix process received help from an AI agent. This bug was first discovered while running Paper; I used an AI agent to trace its origin in Moonrise / CraftBukkit / vanilla Mojang code, and to port the native fix to the mixin framework.
Besides, as I'm not a native English speaker, this PR and the companion issue #191 were polished by LLM.