fix: serialize killchannel map access to prevent concurrent-map race

killchannel (map[string]chan bool) was read, written and deleted from both HTTP
request goroutines (Connect/Disconnect/logout/admin-delete) and the per-session
startClient goroutine with no synchronization, which can crash the process with
"concurrent map read and map write" / "concurrent map writes" under simultaneous
connects.

Guard every map operation with a dedicated mutex via four helpers
(setKillChannel / getKillChannel / deleteKillChannel / signalKill). The lock is
held only around the map access, never while sending on or receiving from a
channel, so a slow or absent receiver cannot block another session. The
startClient loop now reads its channel through getKillChannel each iteration
instead of indexing the map directly. Behaviour is otherwise unchanged; this
complements the buffered-channel fix from #242.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Thiago Bauken

handlers.go

@@ -275,7 +275,7 @@ func (s *server) Connect() http.HandlerFunc {
 		userinfocache.Set(token, v, cache.NoExpiration)
 
 		log.Info().Str("jid", jid).Msg("Attempt to connect")
-		killchannel[txtid] = make(chan bool, 1)
+		setKillChannel(txtid, make(chan bool, 1))
 		go s.startClient(txtid, jid, token, subscribedEvents)
 
 		if t.Immediate == false {
@@ -332,10 +332,7 @@ func (s *server) Disconnect() http.HandlerFunc {
 			responseJson, err := json.Marshal(response)
 
 			clientManager.DeleteWhatsmeowClient(txtid)
-			select {
-			case killchannel[txtid] <- true:
-			default:
-			}
+			signalKill(txtid)
 
 			if err != nil {
 				s.Respond(w, r, http.StatusInternalServerError, err)
@@ -636,10 +633,7 @@ func (s *server) Logout() http.HandlerFunc {
 				} else {
 					log.Info().Str("jid", jid).Msg("Logged out")
 					clientManager.DeleteWhatsmeowClient(txtid)
-					select {
-					case killchannel[txtid] <- true:
-					default:
-					}
+					signalKill(txtid)
 				}
 			} else {
 				if clientManager.GetWhatsmeowClient(txtid).IsConnected() == true {

killchannel_test.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+)
+
+// TestKillChannelHelpers covers the mutex-guarded killchannel helpers that
+// replaced direct map access. The set/get/signal/delete cycle must behave
+// correctly, and concurrent access must not panic. Under `go test -race` this
+// also proves the previous unguarded map access ("concurrent map read and map
+// write" from request + session goroutines) is gone.
+func TestKillChannelHelpers(t *testing.T) {
+	const u = "func-user"
+
+	// set -> get returns the same channel.
+	ch := make(chan bool, 1)
+	setKillChannel(u, ch)
+	got, ok := getKillChannel(u)
+	if !ok || got != ch {
+		t.Fatalf("getKillChannel after set: got=%v ok=%v, want the same channel", got, ok)
+	}
+
+	// signalKill delivers a non-blocking value into the buffered channel.
+	signalKill(u)
+	select {
+	case v := <-ch:
+		if !v {
+			t.Errorf("kill channel delivered %v, want true", v)
+		}
+	default:
+		t.Error("signalKill did not deliver a value")
+	}
+
+	// delete removes the entry; signalKill on a missing entry is a safe no-op.
+	deleteKillChannel(u)
+	if _, ok := getKillChannel(u); ok {
+		t.Error("entry still present after deleteKillChannel")
+	}
+	signalKill(u) // must not panic on a missing entry
+}
+
+// TestKillChannelConcurrent hammers the helpers from many goroutines. The point
+// is the -race build: the old bare-map access raced; the guarded helpers do not.
+func TestKillChannelConcurrent(t *testing.T) {
+	const n = 100
+	var wg sync.WaitGroup
+	for i := 0; i < n; i++ {
+		uid := fmt.Sprintf("race-user-%d", i)
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			setKillChannel(uid, make(chan bool, 1))
+			signalKill(uid)
+			_, _ = getKillChannel(uid)
+			deleteKillChannel(uid)
+		}()
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			_, _ = getKillChannel(uid)
+			signalKill(uid)
+		}()
+	}
+	wg.Wait()
+}

main.go

@@ -73,6 +73,7 @@ var (
 	container        *sqlstore.Container
 	clientManager    = NewClientManager()
 	killchannel      = make(map[string](chan bool))
+	killchannelMu    sync.Mutex
 	userinfocache    = cache.New(5*time.Minute, 10*time.Minute)
 	lastMessageCache = cache.New(24*time.Hour, 24*time.Hour)
 	globalHTTPClient = newSafeHTTPClient()
@@ -82,6 +83,45 @@ var privateIPBlocks []*net.IPNet
 
 const version = "1.0.6"
 
+// killchannel maps a userID to its session goroutine's kill channel. It is
+// accessed from HTTP request goroutines (Connect/Disconnect/logout/delete) and
+// from the per-session startClient goroutine, so every map operation must be
+// serialized through killchannelMu. The helpers below lock only around the map
+// access itself — never while sending on or receiving from a channel — so a
+// slow or absent receiver can never block another session.
+func setKillChannel(userID string, ch chan bool) {
+	killchannelMu.Lock()
+	killchannel[userID] = ch
+	killchannelMu.Unlock()
+}
+
+func getKillChannel(userID string) (chan bool, bool) {
+	killchannelMu.Lock()
+	ch, ok := killchannel[userID]
+	killchannelMu.Unlock()
+	return ch, ok
+}
+
+func deleteKillChannel(userID string) {
+	killchannelMu.Lock()
+	delete(killchannel, userID)
+	killchannelMu.Unlock()
+}
+
+// signalKill delivers a non-blocking kill signal to userID's session goroutine,
+// if one is registered. The channel is buffered (cap 1) so the send never
+// blocks; the default guards a full buffer or a missing entry.
+func signalKill(userID string) {
+	ch, ok := getKillChannel(userID)
+	if !ok {
+		return
+	}
+	select {
+	case ch <- true:
+	default:
+	}
+}
+
 func newSafeHTTPClient() *http.Client {
 	return &http.Client{
 		Timeout: 60 * time.Second,

wmiau.go

@@ -307,7 +307,7 @@ func (s *server) connectOnStartup() {
 			}
 			eventstring := strings.Join(subscribedEvents, ",")
 			log.Info().Str("events", eventstring).Str("jid", jid).Msg("Attempt to connect")
-			killchannel[txtid] = make(chan bool, 1)
+			setKillChannel(txtid, make(chan bool, 1))
 			go s.startClient(txtid, jid, token, subscribedEvents)
 
 			// Initialize S3 client if configured
@@ -568,10 +568,7 @@ func (s *server) startClient(userID string, textjid string, token string, subscr
 					clientManager.DeleteWhatsmeowClient(userID)
 					clientManager.DeleteMyClient(userID)
 					clientManager.DeleteHTTPClient(userID)
-					select {
-					case killchannel[userID] <- true:
-					default:
-					}
+					signalKill(userID)
 				} else if evt.Event == "success" {
 					log.Info().Msg("QR pairing ok!")
 					// Clear QR code after pairing
@@ -655,10 +652,13 @@ func (s *server) startClient(userID string, textjid string, token string, subscr
 		}
 	}
 
-	// Keep connected client live until disconnected/killed
+	// Keep connected client live until disconnected/killed. Read the kill
+	// channel through the mutex-guarded helper each iteration so this read
+	// never races with concurrent map writes/deletes from request goroutines.
 	for {
+		kill, _ := getKillChannel(userID)
 		select {
-		case <-killchannel[userID]:
+		case <-kill:
 			log.Info().Str("userid", userID).Msg("Received kill signal")
 			client.Disconnect()
 			clientManager.DeleteWhatsmeowClient(userID)
@@ -669,7 +669,7 @@ func (s *server) startClient(userID string, textjid string, token string, subscr
 			if err != nil {
 				log.Error().Err(err).Msg(sqlStmt)
 			}
-			delete(killchannel, userID)
+			deleteKillChannel(userID)
 			return
 		default:
 			time.Sleep(1000 * time.Millisecond)
@@ -1440,10 +1440,7 @@ func (mycli *MyClient) myEventHandler(rawEvt interface{}) {
 		log.Info().Str("reason", evt.Reason.String()).Msg("Logged out")
 		defer func() {
 			// Use a non-blocking send to prevent a deadlock if the receiver has already terminated.
-			select {
-			case killchannel[mycli.userID] <- true:
-			default:
-			}
+			signalKill(mycli.userID)
 		}()
 		sqlStmt := `UPDATE users SET connected=0 WHERE id=$1`
 		_, err := mycli.db.Exec(sqlStmt, mycli.userID)