From 4474855bc1b4e43724525fc11a9ac476d5f8a2c3 Mon Sep 17 00:00:00 2001
From: adarshm11 <amlearning2023@gmail.com>
Date: Sat, 6 Jun 2026 01:08:33 -0400
Subject: [PATCH] fix duplicate user goroutine panic

---
 frontend/src/components/room/RoomPage.tsx | 68 ++++++++++++++++++++++-
 handlers/ws.go                            | 11 ++--
 models/room.go                            | 16 +++++-
 services/room.go                          | 19 +++++--
 4 files changed, 99 insertions(+), 15 deletions(-)

diff --git a/frontend/src/components/room/RoomPage.tsx b/frontend/src/components/room/RoomPage.tsx
index 8948f28..78c73ac 100644
--- a/frontend/src/components/room/RoomPage.tsx
+++ b/frontend/src/components/room/RoomPage.tsx
@@ -53,9 +53,54 @@ function RoomPage({ interviewType: propInterviewType }: RoomPageProps) {
   const [connectionState, setConnectionState] = useState<'connected' | 'reconnecting' | 'failed'>('connected');
   const shouldReconnectRef = useRef(true);
 
+  // Duplicate-tab detection: if another tab in this browser already has this
+  // room open (same origin → same userId in localStorage), block joining here
+  // and prompt the user back to their original tab. Without this, both tabs
+  // race to be the active connection for the same userID and the server thrashes.
+  const [duplicateTabDetected, setDuplicateTabDetected] = useState(false);
+  const [tabCheckComplete, setTabCheckComplete] = useState(false);
+  const tabIdRef = useRef<string>(crypto.randomUUID());
+  const isJoinedRef = useRef(false);
+  useEffect(() => { isJoinedRef.current = isJoined; }, [isJoined]);
+
+  // Detect another tab already in this room before we try to join. Sends a
+  // ping on a per-room BroadcastChannel; any already-joined tab answers with
+  // a pong. If we hear a pong within the grace window we flip duplicateTab
+  // and stop the join flow. Otherwise we mark the check complete and the
+  // auto-join effect proceeds normally.
+  useEffect(() => {
+    if (!roomId) return;
+    if (typeof BroadcastChannel === 'undefined') {
+      setTabCheckComplete(true);
+      return;
+    }
+
+    const channel = new BroadcastChannel(`goderpad-room-${roomId}`);
+
+    const onMessage = (e: MessageEvent) => {
+      if (!e.data || e.data.from === tabIdRef.current) return;
+      if (e.data.type === 'ping' && isJoinedRef.current) {
+        channel.postMessage({ type: 'pong', from: tabIdRef.current });
+      } else if (e.data.type === 'pong') {
+        setDuplicateTabDetected(true);
+        setTabCheckComplete(true);
+      }
+    };
+    channel.addEventListener('message', onMessage);
+
+    channel.postMessage({ type: 'ping', from: tabIdRef.current });
+    const timer = setTimeout(() => setTabCheckComplete(true), 300);
+
+    return () => {
+      clearTimeout(timer);
+      channel.removeEventListener('message', onMessage);
+      channel.close();
+    };
+  }, [roomId]);
+
   const handleJoinRoom = async () => {
-    if (!userName.trim() || !roomId) return;
-    
+    if (!userName.trim() || !roomId || duplicateTabDetected) return;
+
     setIsLoading(true);
     const response = await joinRoom(userId, userName, roomId);
     setIsLoading(false);
@@ -104,6 +149,7 @@ function RoomPage({ interviewType: propInterviewType }: RoomPageProps) {
       navigate('/');
       return;
     }
+    if (!tabCheckComplete || duplicateTabDetected) return;
 
     const storedData = localStorage.getItem(`goderpad-cookie-${roomId}`);
     if (!storedData) return;
@@ -142,7 +188,7 @@ function RoomPage({ interviewType: propInterviewType }: RoomPageProps) {
     };
 
     joinWithStoredData();
-  }, [roomId, userId, navigate]);
+  }, [roomId, userId, navigate, tabCheckComplete, duplicateTabDetected]);
 
   // Setup WebSocket connection and handlers when the user successfully joins the room.
   // Automatically reconnects on close/error with exponential backoff. The server-side
@@ -385,6 +431,22 @@ function RoomPage({ interviewType: propInterviewType }: RoomPageProps) {
   }, [ws, isJoined, userId, userName]);
 
   if (!isJoined) {
+    if (duplicateTabDetected) {
+      return (
+        <Popup
+          message="you're already in this room in another tab!"
+          buttonText="ok"
+          isOpen={true}
+          onClickButton={() => {
+            // window.close() only works on tabs the script itself opened, so
+            // it's a best-effort; fall back to navigating home if the browser
+            // blocks it.
+            window.close();
+            setTimeout(() => navigate('/'), 100);
+          }}
+        />
+      );
+    }
     return (<>
       <Popup
         message="sorry, an error occurred trying to join the room"
diff --git a/handlers/ws.go b/handlers/ws.go
index d0cc571..056cc06 100644
--- a/handlers/ws.go
+++ b/handlers/ws.go
@@ -182,12 +182,13 @@ func readBroadcastsFromUser(user *models.User, room *models.Room) {
 func closeUserConnection(user *models.User, room *models.Room) {
 	userID := user.UserID // Save before closing
 
-	// First remove user from room so they don't receive their own leave message
-	room.RemoveUser(userID)
+	// Only remove the map entry if it still points to THIS user. A concurrent
+	// duplicate join (same userID, e.g. the user opened the room in a second
+	// tab) may have already replaced us with a new User+Conn; in that case the
+	// new tab is the live one and we must not evict it or fire user_left.
+	removed := room.RemoveUserIfSame(userID, user)
 
-	// Broadcast user_left to remaining users. Skip if the room is ending
-	// or already torn down to avoid blocking forever or panicking on send.
-	if !room.IsEnded() {
+	if removed && !room.IsEnded() {
 		select {
 		case room.Broadcast <- models.BroadcastMessage{
 			UserID: userID,
diff --git a/models/room.go b/models/room.go
index 26d2a3a..9af0b5a 100644
--- a/models/room.go
+++ b/models/room.go
@@ -120,11 +120,25 @@ func (r *Room) RemoveUser(userID string) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 	if _, exists := r.Users[userID]; exists {
-		delete(r.Users, userID)	
+		delete(r.Users, userID)
 		metrics.RoomUsersTotal.Dec()
 	}
 }
 
+// RemoveUserIfSame deletes the map entry only if it still references the
+// given User pointer. Used by closeUserConnection so a stale reader goroutine
+// from a dropped tab doesn't evict a new tab that just took over the same userID.
+func (r *Room) RemoveUserIfSame(userID string, user *User) bool {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	if current, exists := r.Users[userID]; exists && current == user {
+		delete(r.Users, userID)
+		metrics.RoomUsersTotal.Dec()
+		return true
+	}
+	return false
+}
+
 func (r *Room) CheckUserExists(userID string) (*User, bool) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
diff --git a/services/room.go b/services/room.go
index f78db62..dd15963 100644
--- a/services/room.go
+++ b/services/room.go
@@ -34,16 +34,23 @@ func JoinRoom(userID, name, roomID string) (map[string]any, error) {
 		return nil, models.ErrRoomNotFound
 	}
 
-	// Reconnect path: if the user is already in the room (their old conn
-	// hasn't been cleaned up yet — e.g., the client noticed the drop
-	// before the server's pong-timeout fired), tear down the stale User
-	// so we don't leak its HandleBroadcasts goroutine or hold a dead Conn.
+	// Reconnect / duplicate-tab path: a User entry already exists for this
+	// userID (e.g. client reconnect after a transient drop, or the user
+	// opened the room in a second tab). Tear down the old one so we don't
+	// leak its HandleBroadcasts goroutine or hold a dead Conn.
 	if existing, ok := room.CheckUserExists(userID); ok {
 		if existing.Conn != nil {
+			// There's an active reader goroutine on this Conn. Closing the
+			// Conn wakes its ReadJSON and triggers its deferred
+			// closeUserConnection, which (with RemoveUserIfSame) safely
+			// cleans up the map entry and User channels. Doing those steps
+			// here too would race the reader and double-close the channels.
 			existing.Conn.Close()
+		} else {
+			// No active reader to clean up after itself — do it ourselves.
+			room.RemoveUser(userID)
+			existing.Close()
 		}
-		room.RemoveUser(userID)
-		existing.Close()
 	}
 
 	user := models.CreateUser(userID, name)