Skip to content

fix: network manager event loop flush, compression lookup, and shutdown order - #161

Open
GamingOP69 wants to merge 6 commits into
Wind-Development:masterfrom
GamingOP69:fix/network-flush-and-shutdown
Open

fix: network manager event loop flush, compression lookup, and shutdown order#161
GamingOP69 wants to merge 6 commits into
Wind-Development:masterfrom
GamingOP69:fix/network-flush-and-shutdown

Conversation

@GamingOP69

Copy link
Copy Markdown

Summary

Fixes packet flushing thread safety in NetworkManager, pipeline key lookup during compression setup, sendQueuedPackets error recovery, and clean shutdown ordering in ServerConnection.

Problem & Root Cause

  • NetworkManager.flush() called channel.flush() directly from non-I/O threads (such as the main server tick thread). Netty channel pipelines are not thread-safe for direct off-thread pipeline manipulations, which could lead to unflushed packets sitting in channel buffers.
  • setupCompression looked up the handler with key "decompress" instead of "compress", failing to insert the compression handler in the expected pipeline position.
  • In PlayerConnection.sendQueuedPackets(), if packet serialization threw an exception during queue draining, enableAutomaticFlush() would never get called, permanently stalling the connection's automatic flushing.
  • ServerConnection.stopServer() shut down event loop groups before closing the listening channels, throwing NullPointerException and resource leak warnings on server shutdown.
  • LazyInitVar lacked double-checked locking, allowing race conditions during concurrent initialization of Netty transport groups.

Solution

  • Updated NetworkManager.flush() to execute flushes via channel.eventLoop().execute(...) when called outside the I/O event loop.
  • Fixed the compression pipeline key lookup to "compress".
  • Wrapped sendQueuedPackets() draining in a try-finally block to guarantee enableAutomaticFlush() runs even if an exception occurs.
  • Ensured listening channels are closed before terminating Netty event loop groups in ServerConnection.
  • Added double-checked locking to LazyInitVar with LazyInitVarTest concurrency unit test.

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