fix: Switch server and some data races

pkg/edition/java/netmc/connection.go

@@ -71,6 +71,8 @@ type MinecraftConn interface { // TODO convert to exported struct as this interf
 	PacketWriter
 
 	Reader() Reader // Only use if you know what you are doing!
+	Writer() Writer
+	EnablePlayPacketQueue()
 }
 
 // Closed returns true if the connection is closed.
@@ -247,6 +249,7 @@ func (c *minecraftConn) startReadLoop() {
 }
 
 func (c *minecraftConn) Reader() Reader { return c.rd }
+func (c *minecraftConn) Writer() Writer { return c.wr }
 
 func (c *minecraftConn) SetAutoReading(enabled bool) {
 	c.log.V(1).Info("update auto reading", "enabled", enabled)
@@ -293,7 +296,7 @@ func (c *minecraftConn) bufferNoQueue(packet proto.Packet) error {
 	return c.bufferPacket(packet, false)
 }
 
-func (c *minecraftConn) bufferPacket(packet proto.Packet, queue bool) (err error) {
+func (c *minecraftConn) bufferPacket(packet proto.Packet, canQueue bool) (err error) {
 	if Closed(c) {
 		return ErrClosedConn
 	}
@@ -302,10 +305,15 @@ func (c *minecraftConn) bufferPacket(packet proto.Packet, queue bool) (err error
 			c.closeOnWriteErr(err, "bufferPacket", fmt.Sprintf("%T", packet))
 		}
 	}()
-	if queue && c.playPacketQueue.Queue(packet) {
-		// Packet was queued, don't write it now
-		c.log.V(1).Info("queued packet", "packet", fmt.Sprintf("%T", packet))
-		return nil
+	if canQueue {
+		c.mu.Lock()
+		playPacketQueue := c.playPacketQueue
+		c.mu.Unlock()
+		if playPacketQueue.Queue(packet) {
+			// Packet was queued, don't write it now
+			c.log.V(1).Info("queued packet", "packet", fmt.Sprintf("%T", packet))
+			return nil
+		}
 	}
 	_, err = c.wr.WritePacket(packet)
 	return err
@@ -469,14 +477,26 @@ func (c *minecraftConn) SetState(s *state.Registry) {
 	}
 }
 
+func (c *minecraftConn) EnablePlayPacketQueue() {
+	if c.mu.TryLock() {
+		defer c.mu.Unlock()
+	}
+	c.activatePlayPacketQueue()
+}
+
+// calling function must hold c.mu
+func (c *minecraftConn) activatePlayPacketQueue() {
+	// Activate the play packet queue if not already
+	if c.playPacketQueue == nil {
+		c.playPacketQueue = queue.NewPlayPacketQueue(c.protocol, c.Writer().Direction())
+	}
+}
+
 // ensurePlayPacketQueue ensures the play packet queue is activated or deactivated
 // when the connection enters or leaves the play state. See PlayPacketQueue struct for more info.
 func (c *minecraftConn) ensurePlayPacketQueue(newState state.State) {
 	if newState == state.ConfigState { // state exists since 1.20.2+
-		// Activate the play packet queue if not already
-		if c.playPacketQueue == nil {
-			c.playPacketQueue = queue.NewPlayPacketQueue(c.protocol, c.direction)
-		}
+		c.activatePlayPacketQueue()
 		return
 	}
 

pkg/edition/java/netmc/writer.go

@@ -22,6 +22,7 @@ type Writer interface {
 	Flush() (err error)
 
 	StateChanger
+	Direction() proto.Direction
 }
 
 // NewWriter returns a new packet writer.

pkg/edition/java/proto/packet/clientsettings.go

@@ -8,108 +8,56 @@ import (
 )
 
 type ClientSettings struct {
-	Locale         string // may be empty
-	ViewDistance   byte
-	ChatVisibility int
-	ChatColors     bool
-	Difficulty     bool // 1.7 Protocol
-	SkinParts      byte
-	MainHand       int
-	TextFiltering  bool // 1.17+
-	ClientListing  bool // 1.18+, overwrites server-list "anonymous" mode
+	Locale               string // may be empty
+	ViewDistance         byte
+	ChatVisibility       int
+	ChatColors           bool
+	Difficulty           byte // 1.7 Protocol
+	SkinParts            byte
+	MainHand             int
+	ChatFilteringEnabled bool // 1.17+
+	ClientListingAllowed bool // 1.18+, overwrites server-list "anonymous" mode
 }
 
 func (s *ClientSettings) Encode(c *proto.PacketContext, wr io.Writer) error {
-	err := util.WriteString(wr, s.Locale)
-	if err != nil {
-		return err
-	}
-	err = util.WriteUint8(wr, s.ViewDistance)
-	if err != nil {
-		return err
-	}
-	err = util.WriteVarInt(wr, s.ChatVisibility)
-	if err != nil {
-		return err
-	}
-	err = util.WriteBool(wr, s.ChatColors)
-	if err != nil {
-		return err
-	}
+	w := util.PanicWriter(wr)
+	w.String(s.Locale)
+	w.Byte(s.ViewDistance)
+	w.VarInt(s.ChatVisibility)
+	w.Bool(s.ChatColors)
 	if c.Protocol.LowerEqual(version.Minecraft_1_7_6) {
-		err = util.WriteBool(wr, s.Difficulty)
-		if err != nil {
-			return err
-		}
-	}
-	err = util.WriteUint8(wr, s.SkinParts)
-	if err != nil {
-		return err
+		w.Byte(s.Difficulty)
 	}
+	w.Byte(s.SkinParts)
 	if c.Protocol.GreaterEqual(version.Minecraft_1_9) {
-		err = util.WriteVarInt(wr, s.MainHand)
-		if err != nil {
-			return err
-		}
+		w.VarInt(s.MainHand)
 		if c.Protocol.GreaterEqual(version.Minecraft_1_17) {
-			err = util.WriteBool(wr, s.TextFiltering)
-			if err != nil {
-				return err
-			}
+			w.Bool(s.ChatFilteringEnabled)
 		}
 		if c.Protocol.GreaterEqual(version.Minecraft_1_18) {
-			err = util.WriteBool(wr, s.ClientListing)
-			if err != nil {
-				return err
-			}
+			w.Bool(s.ClientListingAllowed)
 		}
 	}
 	return nil
 }
 
 func (s *ClientSettings) Decode(c *proto.PacketContext, rd io.Reader) (err error) {
-	s.Locale, err = util.ReadString(rd)
-	if err != nil {
-		return err
-	}
-	s.ViewDistance, err = util.ReadUint8(rd)
-	if err != nil {
-		return err
-	}
-	s.ChatVisibility, err = util.ReadVarInt(rd)
-	if err != nil {
-		return err
-	}
-	s.ChatColors, err = util.ReadBool(rd)
-	if err != nil {
-		return err
-	}
+	r := util.PanicReader(rd)
+	r.StringMax(&s.Locale, 16)
+	r.Byte(&s.ViewDistance)
+	r.VarInt(&s.ChatVisibility)
+	r.Bool(&s.ChatColors)
 	if c.Protocol.LowerEqual(version.Minecraft_1_7_6) {
-		s.Difficulty, err = util.ReadBool(rd)
-		if err != nil {
-			return err
-		}
-	}
-	s.SkinParts, err = util.ReadByte(rd)
-	if err != nil {
-		return err
+		r.Byte(&s.Difficulty)
 	}
+	r.Byte(&s.SkinParts) // Go bytes are unsigned already
 	if c.Protocol.GreaterEqual(version.Minecraft_1_9) {
-		s.MainHand, err = util.ReadVarInt(rd)
-		if err != nil {
-			return err
-		}
+		r.VarInt(&s.MainHand)
 		if c.Protocol.GreaterEqual(version.Minecraft_1_17) {
-			s.TextFiltering, err = util.ReadBool(rd)
-			if err != nil {
-				return err
-			}
+			r.Bool(&s.ChatFilteringEnabled)
 		}
 		if c.Protocol.GreaterEqual(version.Minecraft_1_18) {
-			s.ClientListing, err = util.ReadBool(rd)
-			if err != nil {
-				return err
-			}
+			r.Bool(&s.ClientListingAllowed)
 		}
 	}
 	return nil

pkg/edition/java/proto/state/states.go

@@ -200,6 +200,8 @@ func init() {
 		m(0x27, version.Minecraft_1_20_2),
 		m(0x28, version.Minecraft_1_20_3),
 	)
+	Play.ServerBound.Register(&config.FinishedUpdate{},
+		m(0x0B, version.Minecraft_1_20_2))
 
 	Play.ClientBound.Register(&p.KeepAlive{},
 		m(0x00, version.Minecraft_1_7_2),
@@ -459,5 +461,4 @@ func init() {
 		m(0x65, version.Minecraft_1_20_2),
 		m(0x67, version.Minecraft_1_20_3),
 	)
-
 }

pkg/edition/java/proto/util/preader.go

@@ -18,6 +18,14 @@ func (r *PReader) String(s *string) {
 	PReadString(r.r, s)
 }
 
+func (r *PReader) StringMax(s *string, max int) {
+	PReadStringMax(r.r, s, max)
+}
+
+func (r *PReader) Uint8(i *uint8) {
+	PReadUint8(r.r, i)
+}
+
 func (r *PReader) Bytes(b *[]byte) {
 	PReadBytes(r.r, b)
 }
@@ -95,6 +103,22 @@ func PReadString(rd io.Reader, s *string) {
 	*s = v
 }
 
+func PReadStringMax(rd io.Reader, s *string, max int) {
+	v, err := ReadStringMax(rd, max)
+	if err != nil {
+		panic(err)
+	}
+	*s = v
+}
+
+func PReadUint8(rd io.Reader, i *uint8) {
+	v, err := ReadUint8(rd)
+	if err != nil {
+		panic(err)
+	}
+	*i = v
+}
+
 func PReadBytes(rd io.Reader, b *[]byte) {
 	v, err := ReadBytes(rd)
 	if err != nil {

pkg/edition/java/proxy/bungee.go

@@ -14,7 +14,7 @@ import (
 	"go.minekube.com/gate/pkg/gate/proto"
 )
 
-func bungeeCordMessageResponder(
+func newBungeeCordMessageResponder(
 	bungeePluginChannelEnabled bool,
 	player *connectedPlayer,
 	proxy *Proxy,

pkg/edition/java/proxy/player.go

@@ -706,12 +706,15 @@ func (p *connectedPlayer) config() *config.Config {
 
 // switchToConfigState switches the connection of the client into config state.
 func (p *connectedPlayer) switchToConfigState() {
-	go func() {
-		if err := p.WritePacket(new(cfgpacket.StartUpdate)); err != nil {
-			p.log.Error(err, "error writing config packet")
-		}
-		p.SetState(state.Config)
-	}()
+	if err := p.BufferPacket(new(cfgpacket.StartUpdate)); err != nil {
+		p.log.Error(err, "error writing config packet")
+	}
+
+	p.MinecraftConn.Writer().SetState(state.Config)
+	// Make sure we don't send any play packets to the player after update start
+	p.MinecraftConn.EnablePlayPacketQueue()
+
+	_ = p.Flush() // Trigger switch finally
 }
 
 func (p *connectedPlayer) ClientBrand() string {

pkg/edition/java/proxy/player/settings.go

@@ -75,7 +75,7 @@ type clientSettings struct {
 	s      *packet.ClientSettings
 }
 
-func (s *clientSettings) ClientListing() bool { return s.s.ClientListing }
+func (s *clientSettings) ClientListing() bool { return s.s.ClientListingAllowed }
 
 func (s *clientSettings) SkinParts() SkinParts {
 	return SkinParts(s.s.SkinParts)