Skip to content

[Extension] connect() async gap can create duplicate real WebSocket connections #1552

Description

@jackwener

Background

Found during PR #1540 review (race-condition fix for stale WebSocket message clobber).

In extension/src/background.ts connect():

async function connect() {
  if (ws?.readyState === WebSocket.OPEN || ws?.readyState === WebSocket.CONNECTING) return;  // L103
  // ...
  try {
    const contextId = await getCurrentContextId();  // L114 — async gap
    thisWs = new WebSocket(DAEMON_WS_URL);
    ws = thisWs;
    currentContextId = contextId;
  }
}

Caller sources: keepalive alarm (~24s), scheduleReconnect() setTimeout, initial startup. If a second connect() enters during the await at L114, both pass L103 and both new WebSocket() → two real TCP connections to daemon.

PR #1540 makes the "losing" socket's onopen/onclose/onmessage short-circuit so behavior is correct — but the extra socket still consumes a daemon connection slot until natural timeout / daemon kicks it.

Proposed fix

In-flight lock (mutex pattern):

let connecting: Promise<void> | null = null;
async function connect() {
  if (connecting) return connecting;
  if (ws?.readyState === WebSocket.OPEN || ws?.readyState === WebSocket.CONNECTING) return;
  connecting = (async () => {
    try { /* existing body */ } finally { connecting = null; }
  })();
  return connecting;
}

Acceptance

  • Concurrent connect() calls during async gap → only one WebSocket created
  • Regression test mocking concurrent calls + slow getCurrentContextId()
  • No behavior change for non-concurrent path

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions