Skip to content

Fix shared static variables in structure generation - #14175

Draft
lisolaris wants to merge 1 commit into
PaperMC:mainfrom
lisolaris:fix/structure-generation-race
Draft

Fix shared static variables in structure generation#14175
lisolaris wants to merge 1 commit into
PaperMC:mainfrom
lisolaris:fix/structure-generation-race

Conversation

@lisolaris

@lisolaris lisolaris commented Aug 10, 2026

Copy link
Copy Markdown

Problem

Fixes #14176: a parallel structure generation race in NetherFortressPieces and StrongholdPieces: their piece-weight selection state lives in shared static fields, which is safe only because vanilla structure generation assumes serialized execution. STRUCTURE_STARTS is executed in parallel, so two fortresses (or strongholds) generating on different worker threads can corrupt each other's state — nether fortresses get prematurely sealed off or abnormally extended, strongholds can lose their portal room.

Changes

  • NetherFortressPieces: the StartPiece constructor copies each PieceWeight instead of resetting and sharing the static array elements — the static arrays are never mutated after class loading.
  • StrongholdPieces: the static currentPieces / totalWeight / imposedPiece / PieceWeight.placeCount state moves to per-StartPiece instance fields; placeCount is tracked in an IdentityHashMap keyed by the shared weight object. A minDepth field is added to PieceWeight to replace the doPlace overrides in the Library / PortalRoom anonymous subclasses, so the standard weight logic handles the depth constraints directly rather than copying the weights.
  • StrongholdStructure: the now-redundant resetPieces() call is removed (the reset happens in the StartPiece constructor).

Why IdentityHashMap?

Counts are keyed by the shared weight object's identity — the same semantics as vanilla's PieceWeight.placeCount, which lives on the object itself. It does not rely on equals/hashCode, so the counting is unaffected if PieceWeight ever overrides them.

Why not ThreadLocal?

Paper can modify the vanilla classes directly, so ownership is expressed naturally as per-StartPiece state instead of redirecting accesses through ThreadLocal (which the Moonrise fix uses because its mixin handlers cannot reach the StartPiece local — see Tuinity/Moonrise#192). The state access points either already take a StartPiece parameter (generatePieceFromSmallDoor) or are parameterised trivially (updatePieceWeight), so no signature chains change.

Semantics

Only the state access location changes — no nextInt call order or evaluation order is touched, so single-threaded generation stays bit-for-bit identical to vanilla.

Verification

The same seed (5535345) was tested on vanilla server, Paper before the fix, and Paper after the fix. The relevant comparison is Paper after vs vanilla (not Paper after vs Paper before).

  • Seed 5535345: the control fortress [18,17] (generated alone) is identical to vanilla piece-for-piece; the two concurrently generated fortresses [-6,-5], [-5,7] now match the vanilla tree (castle sections present, seal-off ratio back to vanilla levels).
  • Stronghold: regression check (instrumented, 1144 events) — no behavioural differences from vanilla were observed: PortalRoom exactly once per structure, the imposedPieceFiveCrossing chain intact. Note this is a regression check only: the stronghold race cannot be exercised in normal play (sparse start positions) and was not reproduced, so the stronghold fix is preventive — validated by the identical shared-state pattern rather than by observing the failure.
  • Cross-platform: the companion Moonrise fix (same semantics) was verified on Fabric and NeoForge with no behavioural differences.

Why a separate fix in Paper

Moonrise and Paper have different integration models:

  • Moonrise uses runtime mixins;
  • Paper applies source patches.

Therefore this fix is implemented independently (a corresponding fix has been prepared in Moonrise) while preserving identical behaviour — 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, including full code research for the structure generation race, debug log instrumentation, and specific code changes.

Besides, as I'm not a native English speaker, this PR and the companion issue #14176 were polished by LLM.

Structure starts can be generated in parallel (STRUCTURE_STARTS is marked
parallel-capable), but NetherFortressPieces and StrongholdPieces keep their
piece-weight state (placeCount, currentPieces, imposedPiece, totalWeight) in
shared static fields. Two structures generating concurrently corrupt each
other's state: nether fortresses get prematurely sealed off or abnormally
extended, strongholds can lose their portal room.

Move the state to per-StructureStart instances: the nether fortress copies
its weight objects in the StartPiece constructor, and the stronghold tracks
per-structure state in StartPiece fields (IdentityHashMap placeCounts,
minDepth replaces the anonymous-subclass depth constraints).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Awaiting review

Development

Successfully merging this pull request may close these issues.

Parallel structure generation corrupts nether fortresses / strongholds

1 participant