Skip to content

Fix set_entity_data disconnects, overheating damage in sealed rooms, and a CraterCarver data race - #130

Merged
RealAntEngineer merged 3 commits into
RealAntEngineer:1.21.1-releasefrom
dhm-msd:pr/1-bug-fixes
Oct 19, 2026
Merged

RealAntEngineer merged 3 commits into
RealAntEngineer:1.21.1-releasefrom
dhm-msd:pr/1-bug-fixes

Conversation

@dhm-msd

@dhm-msd dhm-msd commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three small, independent bug fixes (one commit each). Together they fix random player disconnects on busy servers and two smaller issues I found while reading the code.

1. Disconnect: ConcurrentModificationException while encoding set_entity_data

On our SMP, players got kicked while in a sealed base on the moon (the server was lagging, "Can't keep up, 91 ticks behind"):

EncoderException: Failed to encode packet 'clientbound/minecraft:set_entity_data'
Caused by: java.util.ConcurrentModificationException
at java.util.ArrayList$Itr.next
at ByteBufCodecs$21.encode (ByteBufCodecs.java:373)
at SynchedEntityData$DataValue.write
at ClientboundSetEntityDataPacket.pack / write

Cause: RoomShapeSerializer.copy() returned the same RoomShape, so the packet held a reference to the live box list. RoomAtmosphere.searchTopology() edited that list in place, while the packet is encoded later on a Netty thread. A rebuild at the wrong moment throws and the tracking player is disconnected. Lag makes it far more likely. It does not happen in single player, because the host player's connection never goes through the packet encoder.

Fix: a published RoomShape is never modified. searchTopology builds a new list and a new shape on every rebuild, so copy() can return the same instance safely: no lock and no copy per sync. The one list copy per rebuild replaces the old in-place edit. (I considered a mutex, but this avoids contention with the Netty thread and the per-sync allocation.)

2. Overheating damage inside sealed rooms

inO2() consumes the "inside an oxygen room" flag, and the suffocation and overheating checks each called it. The second call always said "no oxygen", so entities in a pressurised room took overheating damage. It is now evaluated once per check.

3. CraterCarver.heightMap thread safety

The carver is shared by all chunk-generation workers but its cache was a plain HashMap. It is now a ConcurrentHashMap.

How to reproduce

It needs a real network connection: the host player in single player never goes through the packet encoder, so it doesn't happen there.

  1. Run a dedicated server and join it from a separate client.
  2. Build a sealed room with an oxygen sealer (the room entity must be tracked by the player).
  3. Use a Create wrench on a pipe next to the room.

Result on 1.7.23: the player is disconnected right away with the trace above (Failed to encode packet 'clientbound/minecraft:set_entity_data' ← ConcurrentModificationException). A wrench click is enough because it fires a block update, and the code then rebuilds the room immediately on the server thread, editing the shape's box list in place while the Netty thread may be encoding it.

Testing

  • Reproduced the disconnect on a dedicated server with the original jar (1 wrench click, trace in the server log).
  • With this change, the same steps ran without a disconnect or any set_entity_data error in the log.

Notes

  • No behaviour change beyond the fixes above.
  • Followed by two more PRs (performance and room simulation). pr/3 builds on this one.

…ackets

RoomShapeSerializer.copy() returned the same instance, so the set_entity_data packet
kept a reference to the live box list. searchTopology edited that list in place while
the packet was encoded later on a Netty thread, causing a ConcurrentModificationException
-> EncoderException and a disconnect for the tracking player.

A published RoomShape is now never modified: searchTopology builds a new list and a new
shape on every rebuild, so the shape can be shared with packets without a lock or a copy
per sync. The one list copy per rebuild replaces the in-place edit.
inO2() consumes the 'inside an oxygen room' flag, and the suffocation and overheating
checks each called it. The second call always reported 'no oxygen', so entities standing
in a pressurised room took overheating damage. Evaluate it once per check and run the
overheating branch only when there is no oxygen.
The carver instance is shared by all chunk-generation worker threads, but its height
cache was a plain HashMap, which can corrupt or loop under concurrent access. Use a
ConcurrentHashMap.
@CLAassistant

CLAassistant commented Oct 19, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dhm-msd
dhm-msd marked this pull request as ready for review October 19, 2026 20:20
@RealAntEngineer

Copy link
Copy Markdown
Owner

Do you use AI ? (simple question, no juging)

@RealAntEngineer
RealAntEngineer merged commit 0f94fa5 into RealAntEngineer:1.21.1-release Oct 19, 2026
2 checks passed
@dhm-msd

dhm-msd commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Yes! With lots of tests and review of course.

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.

3 participants