Skip to content

fix(recording): reschedule packet send recording onto server thread#45

Merged
JustinDevB merged 1 commit into
JustinDevB:devfrom
DriftN2Forty:fix/issue-43-netty-thread-safety
May 1, 2026
Merged

fix(recording): reschedule packet send recording onto server thread#45
JustinDevB merged 1 commit into
JustinDevB:devfrom
DriftN2Forty:fix/issue-43-netty-thread-safety

Conversation

@DriftN2Forty
Copy link
Copy Markdown
Collaborator

Summary

  • reschedule block break packet recording onto the server thread before any Bukkit lookups or recording-state mutation
  • capture only the packet fields needed from the PacketEvents callback so the Netty-side work stays narrow
  • add regression coverage for the handoff path and document the fix in the changelog

Problem

Issue #43 reported a Netty packet-send thread blocked in HashMap.put while BetterReplay was recording block break animations.

Before this change, the PacketEvents send listener handled more than packet decoding. It also performed Bukkit lookups and mutated shared recording state directly from the packet callback. That meant a Netty thread could touch:

  • the block break dedup map
  • tracked player and entity state
  • the timeline builder and append-log path

That is the wrong thread boundary for both Bukkit access and BetterReplay's recording session state.

Fix

The packet listener now copies the minimal block break animation data it needs immediately, then reschedules the actual recording work onto the server thread through FoliaLib. Bukkit lookups, dedup mutation, and timeline event emission all happen on the server thread again.

This keeps the packet callback small and removes the cross-thread mutation hazard that was behind issue #43.

Validation

  • runTests passed for RecordingPacketHandlerTest
  • the full test suite passed with 481 tests green

Refs #43

Issue JustinDevB#43 reported a Netty packet-send thread blocked in HashMap.put while BetterReplay
was recording block break animations. The packet callback was doing Bukkit lookups and
mutating shared recording state directly from PacketEvents' send listener, which meant
Netty could touch the block-break dedup map, timeline builder, entity tracking, and
append-log path off the server thread.

Capture the packet data immediately, then hand the recording work back to the server
thread through FoliaLib before reading Bukkit state or mutating recording structures.
This keeps the PacketEvents callback narrow and removes the cross-thread mutation hazard
that could stall shutdown and contend inside HashMap tree insertion.

Add regression coverage for the main-thread handoff and for tracked-viewer block break
stage recording, and document the fix in the changelog.

Validation: runTests passed for RecordingPacketHandlerTest and the full suite passed
with 481 tests green.

Refs: JustinDevB#43
@JustinDevB JustinDevB merged commit 46bb99d into JustinDevB:dev May 1, 2026
1 check passed
@DriftN2Forty DriftN2Forty deleted the fix/issue-43-netty-thread-safety branch May 1, 2026 02:37
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.

2 participants