From 3f1e5d42ad7b2837032272645241b311f66539fd Mon Sep 17 00:00:00 2001
From: "Brandon McGinty (chatgpt)" <git@bmcginty.us>
Date: Sun, 9 Aug 2026 14:21:22 -0400
Subject: [PATCH] Avoid deadlock on unknown user channel

---
 fix.txt                                   |  4 ++--
 gumble/gumble/handlers.go                 |  2 +-
 gumble/gumble/handlers_regression_test.go | 28 +++++++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 gumble/gumble/handlers_regression_test.go

diff --git a/fix.txt b/fix.txt
index d139e61..4abaf26 100644
--- a/fix.txt
+++ b/fix.txt
@@ -37,14 +37,14 @@ Priority 0: security and crashers
    nil. Initialize the map in DialWithDialer and assign its owning client when
    actions are created. Add handler tests for add/remove/trigger.
 
-4. Unknown ChannelId deadlocks the protocol reader
+[x] 4. Unknown ChannelId deadlocks the protocol reader
    File: gumble/gumble/handlers.go
    In handleUserState, the unknown ChannelId branch takes c.volatile.Lock()
    again instead of unlocking before returning. This leaves the mutex locked
    forever. Replace with one unlock (prefer defer after acquisition) and add
    a malformed/out-of-order channel test.
 
-5. OpenAL Buffer.Delete deletes a source, not a buffer
+[x] 5. OpenAL Buffer.Delete deletes a source, not a buffer
    File: gumble/go-openal/openal/buffer.go
    Buffer.Delete calls C.walDeleteSource. It must call C.walDeleteBuffer.
    The current code reports invalid source errors and leaks OpenAL buffers.
diff --git a/gumble/gumble/handlers.go b/gumble/gumble/handlers.go
index 0cc859f..5f33c37 100644
--- a/gumble/gumble/handlers.go
+++ b/gumble/gumble/handlers.go
@@ -660,7 +660,7 @@ func (c *Client) handleUserState(buffer []byte) error {
 			}
 			newChannel := c.Channels[*packet.ChannelId]
 			if newChannel == nil {
-				c.volatile.Lock()
+				c.volatile.Unlock()
 				return errInvalidProtobuf
 			}
 			if newChannel != user.Channel {
diff --git a/gumble/gumble/handlers_regression_test.go b/gumble/gumble/handlers_regression_test.go
new file mode 100644
index 0000000..f1a86d4
--- /dev/null
+++ b/gumble/gumble/handlers_regression_test.go
@@ -0,0 +1,28 @@
+package gumble
+
+import (
+	"testing"
+	"time"
+
+	"git.stormux.org/storm/barnard/gumble/gumble/MumbleProto"
+	"google.golang.org/protobuf/proto"
+)
+
+// Regression: an out-of-order user move to an unknown channel used to lock
+// volatile twice, permanently deadlocking subsequent protocol handling.
+func TestUserStateUnknownChannelDoesNotDeadlock(t *testing.T) {
+	c := &Client{Config: NewConfig(), Users: make(Users), Channels: make(Channels)}
+	c.Users.create(1)
+	id, channel := uint32(1), uint32(99)
+	data, _ := proto.Marshal(&MumbleProto.UserState{Session: &id, ChannelId: &channel})
+	if err := c.handleUserState(data); err != errInvalidProtobuf {
+		t.Fatalf("got %v", err)
+	}
+	locked := make(chan struct{})
+	go func() { c.volatile.Lock(); c.volatile.Unlock(); close(locked) }()
+	select {
+	case <-locked:
+	case <-time.After(time.Second):
+		t.Fatal("volatile lock was left locked")
+	}
+}