Skip to content

Fix parallel structure generation race in NetherFortressPieces and StrongholdPieces - #192

Open
lisolaris wants to merge 1 commit into
Tuinity:mc/26.2from
lisolaris:fix/parallel-structure-generate
Open

Fix parallel structure generation race in NetherFortressPieces and StrongholdPieces#192
lisolaris wants to merge 1 commit into
Tuinity:mc/26.2from
lisolaris:fix/parallel-structure-generate

Conversation

@lisolaris

@lisolaris lisolaris commented Aug 10, 2026

Copy link
Copy Markdown

Problem

Fixes the race reported in #191 — parallel STRUCTURE_STARTS generation corrupting nether fortresses and strongholds. See the issue for reproduction and evidence.

Changes

  • NetherFortressPieces$StartPieceMixin: the StartPiece constructor now copies each PieceWeight instead of resetting and sharing the static array elements; the vanilla placeCount = 0 reset is discarded (the copies start at 0). The static arrays are never mutated after class loading. The List.add redirect guards against future non-PieceWeight additions (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-thread StrongholdState (patches/structure/StrongholdState.java, a plain helper class shared by both mixins).
  • Access widener: opens NetherFortressPieces$PieceWeight, StrongholdPieces$PieceWeight, StrongholdPieces$StrongholdPiece and STRONGHOLD_PIECE_WEIGHTS for the mixins to reference (compiled in via loom for Fabric; converted to an access transformer for NeoForge).

Unlike NetherFortressPieces, the StrongholdPieces weights cannot be copied per structure: the Library / PortalRoom entries 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 isolated StrongholdState, and placeCount is tracked in an IdentityHashMap keyed by the shared weight object; the depth constraints are re-implemented via pieceClass checks, 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.generatePieces calls resetPieces() before constructing each StartPiece (once per do-while retry), and the @Overwrited resetPieces() clears the thread's state — it rebuilds currentPieces, and clears placeCounts and imposedPiece. 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-StartPiece copies.

Semantics

Only the state access location changes — no nextInt call order or evaluation order is touched. Only ownership changes; algorithmic behaviour does not. Single-threaded generation is bit-for-bit identical to vanilla.

Verification

  • MixinAuditTest passes on both Fabric and NeoForge (all mixins apply cleanly).
  • Nether fortress, seed 5535345 (instrumented before/after builds on both loaders):
    • BEFORE: shared weight identity across threads, interleaved counter increments (e.g. 8→10 skipping 9), premature seal-off (27 pieces vs vanilla 121, castle sections never start, seal-off ratio ~4× vanilla).
    • AFTER: the control fortress [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).
  • Stronghold regression, seed 5535345 (instrumented, both loaders): 1367 events; no behavioural differences from vanilla were observed — PortalRoom exactly once per structure (tries=1), first piece FiveCrossing (the imposedPiece chain that would break if the state were not shared correctly), and unchanged NULL / FillerCorridor seal-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

  • The race also affects Paper (same scheduler). Paper is a source-patch project, so its fix can modify the vanilla classes directly and will use a different implementation (per-StructureStart instantiation 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.
  • Because adjacent stronghold starts are far apart (≥ ~1800 blocks), concurrent generation appears extremely unlikely in practice — this fix is defensive for strongholds: it eliminates the race carrier based on the shared-state analysis, but the race itself was not observed (and we did not force it). The regression check above confirms stronghold generation behaviour is unchanged. The nether fortress corruption, by contrast, is directly reproducible.

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.

…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
lisolaris force-pushed the fix/parallel-structure-generate branch from 7490830 to 01cba2f Compare August 10, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant