Skip to content

fix(vnc-watcher): detect Xvfb display when launched with -displayfd - #5243

Closed
paranoidi wants to merge 1 commit into
jo-inc:masterfrom
paranoidi:fix/vnc-watcher-displayfd-detection
Closed

fix(vnc-watcher): detect Xvfb display when launched with -displayfd#5243
paranoidi wants to merge 1 commit into
jo-inc:masterfrom
paranoidi:fix/vnc-watcher-displayfd-detection

Conversation

@paranoidi

Copy link
Copy Markdown
Contributor

Disclaimer: PR made with AI but I verified locally that it fixes the issue for me.

Bug Description

When Camofox runs with VNC enabled (ENABLE_VNC=1), noVNC fails with "Failed to connect to 127.0.0.1:5900: Connection refused" because x11vnc never starts. The VNC watcher script loops forever unable to detect the Xvfb display.
Root Cause

The VNC watcher script uses the awk regex /\Xvfb :[0-9]+/ to find Xvfb, which only matches when Xvfb is launched with a hardcoded display number (e.g. Xvfb :0 -screen ...). The camoufox-js VirtualDisplay class launches Xvfb with -displayfd 3 (dynamic display assignment), so the command line looks like Xvfb -displayfd 3 -screen ... — no :0 in argv. The watcher never finds it, x11vnc never attaches, and noVNC stays permanently disconnected.
Fix

Replace the inline awk detection with a detect_display() function that tries three fallbacks in order:

  1. Xvfb with hardcoded :N display in argv (original method, regex relaxed to /\Xvfb /)
  2. /tmp/.X*-lock files — the traditional X lock file that contains the display number
  3. /tmp/.X* sockets — fallback via /tmp/.X11-unix/X* files when no lock file exists

How to Verify

  1. Start with ENABLE_VNC=1: docker run -d -p 9377:9377 -p 6080:6080 -p 5901:5900 -e ENABLE_VNC=1 camofox-browser:135.0.1-x86_64
  2. Wait for the browser to pre-warm (check curl http://localhost:9377/health for browserConnected:true)
  3. Open http://localhost:6080/vnc.html in a browser and click Connect
  4. Confirm you see the Camofox browser window live

The VNC watcher's awk regex /:Xvfb :[0-9]+/ only matched Xvfb
launched with a hardcoded display number (e.g. 'Xvfb :0'). The
camoufox-js VirtualDisplay class uses -displayfd 3 (dynamic display
assignment), so the watcher never found the display and x11vnc never
started, causing noVNC to fail with 'Connection refused'.

Replace the inline awk with a detect_display() function that tries
three fallbacks:
1. Xvfb with hardcoded :N display in argv
2. /tmp/.X*-lock files (traditional lock file)
3. /tmp/.X11-unix/X* sockets

With the fix, the watcher successfully detects ':0' in -displayfd
mode and attaches x11vnc, making noVNC work.

Guard x11vnc command with || true so set -e doesn't kill watcher.
Remove set -e so watcher survives x11vnc failures.
@gearwave00001

gearwave00001 commented Jul 2, 2026

Copy link
Copy Markdown

This was also an issue for me with noVNC.html and was resolved in a similar way. My agent performed similar fixes plus a few additions so I ran it against your PR to combine.

Here's the full diff against the original vnc-watcher.sh:

diff
 #!/bin/sh
VNC watcher: detects Camoufox's dynamically-assigned Xvfb display and attaches
x11vnc + noVNC to it. Handles browser restarts (re-attaches on display change).

 set -e

 VNC_PORT="${VNC_PORT:-5900}"
 NOVNC_PORT="${NOVNC_PORT:-6080}"
 VNC_RESOLUTION="${VNC_RESOLUTION:-1920x1080x24}"

 log() { printf '[vnc-watcher] %s\n' "$*" >&2; }

 CURRENT_DISPLAY=""
 X11VNC_PID=""

Prepare password file if requested
 PASSFILE=""
 if [ -n "${VNC_PASSWORD:-}" ]; then
   mkdir -p /tmp/.vnc
   x11vnc -storepasswd "$VNC_PASSWORD" /tmp/.vnc/passwd >/dev/null 2>&1
   PASSFILE="/tmp/.vnc/passwd"
   log "x11vnc: password protected"
 else
   log "x11vnc: NO password (bind $NOVNC_PORT to $VNC_BIND on host)"
 fi

Start noVNC (websockify)
 NOVNC_DIR="/usr/share/novnc"
 if [ ! -d "$NOVNC_DIR" ]; then
   log "ERROR: $NOVNC_DIR not found; noVNC cannot start"
   exit 1
 fi
 VNC_BIND="${VNC_BIND:-127.0.0.1}"
 log "Starting noVNC (websockify) on $VNC_BIND:$NOVNC_PORT -> 127.0.0.1:$VNC_PORT"
 websockify --web "$NOVNC_DIR" "$VNC_BIND:$NOVNC_PORT" "127.0.0.1:$VNC_PORT" >/var/log/novnc.log 2>&1 &

+# detect_display - tries multiple methods to find the Xvfb display number
+detect_display() {
+  # Method 1: Xvfb with hardcoded :N in argv (original method)
+  _found=$(ps -eo args= 2>/dev/null | awk '/\/Xvfb / { for(i=1;i<=NF;i++) if($i ~ /^:[0-9]+$/) {print $i; exit} }' | head -1)
+  if [ -n "$_found" ]; then
+    echo "$_found"
+    return
+  fi
+
+  # Method 2: /tmp/.X*-lock files (works with -displayfd mode)
+  for _lock in /tmp/.X*-lock; do
+    if [ -f "$_lock" ]; then
+      _num=$(basename "$_lock" | sed 's/^\.X//')
+      echo ":$_num"
+      return
+    fi
+  done
+
+  # Method 3: /tmp/.X11-unix/X* sockets
+  for _sock in /tmp/.X11-unix/X*; do
+    if [ -S "$_sock" ] || [ -e "$_sock" ]; then
+      _num=$(basename "$_sock" | sed 's/^X//')
+      echo ":$_num"
+      return
+    fi
+  done
+}

 log "VNC watcher started -- will attach x11vnc when Camoufox's Xvfb appears"

 while true; do
-  # Find Xvfb with our patched resolution
-  FOUND=$(ps -eo args= 2>/dev/null | awk -v res="$VNC_RESOLUTION" '
-    /\/Xvfb :[0-9]+/ && index($0, res) {
-      for (i=1;i<=NF;i++) if ($i ~ /^:[0-9]+$/) { print $i; exit }
-    }
-  ' | head -1)
+  FOUND=$(detect_display)

   if [ -n "$FOUND" ] && [ "$FOUND" != "$CURRENT_DISPLAY" ]; then
     # New or changed display -- (re)attach x11vnc
     if [ -n "$X11VNC_PID" ] && kill -0 "$X11VNC_PID" 2>/dev/null; then
       log "Camoufox display changed ($CURRENT_DISPLAY -> $FOUND), restarting x11vnc"
       kill "$X11VNC_PID" 2>/dev/null || true
       sleep 0.5
     fi

     CURRENT_DISPLAY="$FOUND"
     log "Attaching x11vnc to DISPLAY=$CURRENT_DISPLAY"

+    # Poll xdpyinfo until the display accepts connections
+    for _wait in 1 2 3 4 5; do
+      if xdpyinfo -display "$CURRENT_DISPLAY" >/dev/null 2>&1; then
+        break
+      fi
+      sleep 1
+    done

-    X11VNC_ARGS="-display $CURRENT_DISPLAY -forever -shared -rfbport $VNC_PORT -noxdamage -quiet -bg -o /var/log/x11vnc.log"
+    X11VNC_ARGS="-display $CURRENT_DISPLAY -forever -shared -rfbport $VNC_PORT -quiet -bg -o /var/log/x11vnc.log"
     [ "${VIEW_ONLY:-0}" = "1" ] && X11VNC_ARGS="$X11VNC_ARGS -viewonly"
     if [ -n "$PASSFILE" ]; then
       X11VNC_ARGS="$X11VNC_ARGS -rfbauth $PASSFILE"
     else
       X11VNC_ARGS="$X11VNC_ARGS -nopw"
     fi

-    x11vnc $X11VNC_ARGS || true
+    x11vnc $X11VNC_ARGS || {
+      log "x11vnc failed to start, retrying in 2 seconds..."
+      sleep 2
+      x11vnc $X11VNC_ARGS || true
+    }
     sleep 1
     X11VNC_PID=$(pgrep -f "x11vnc.*-display $CURRENT_DISPLAY" | head -1)
     log "x11vnc running (pid=$X11VNC_PID) on DISPLAY=$CURRENT_DISPLAY"
   fi

   sleep 2
 done

From PR #5243 (paranoidi):

  • detect_display() function with 3 fallback methods (argv, lock files, X11-unix sockets)
  • Replaces the broken awk regex //Xvfb :[0-9]+/ that only matched hardcoded :0

My additions:

  • xdpyinfo polling loop — waits up to 5 seconds for X display to accept connections before starting x11vnc (prevents XIO error crash)
  • x11vnc retry logic — if first attempt fails, retries once after 2 seconds
  • Removed -noxdamage flag (can cause issues with some X servers)
  • Fixed basename parsing for /tmp/.X11-unix/X* sockets (was returning :X0 instead of :0)
  • Replaced local bash-isms with prefixed variables (_found, _num, etc.) for dash compatibility

gearwave00001 added a commit to gearwave00001/camofox-browser that referenced this pull request Jul 9, 2026
…info polling

Three bugs caused noVNC to show 'failed to connect to server' after the
browser goes idle and restarts:

1. Display detection — Xvfb launched with -displayfd 3 has no display
   number in argv, so the original awk regex never matched. Replace with
   detect_display() that falls back to /tmp/.X*-lock and /tmp/.X11-unix/X*
   sockets.

2. XIO error crash — x11vnc connects before Xvfb is fully ready and
   crashes silently (masked by -bg). Poll xdpyinfo for up to 5 seconds
   before starting x11vnc. Also remove -noxdamage which can cause issues
   with some Xvfb configs.

3. Stale PID tracking — After x11vnc dies, CURRENT_DISPLAY stays set so
   the attach block is skipped on every subsequent loop. Add a heartbeat
   check at the top of each loop that detects dead x11vnc and clears
   CURRENT_DISPLAY for re-attach. Also clear on pgrep failure after start.

Remove set -e so the watcher survives x11vnc failures. Replace with
|| true guards and explicit retry logic.

Based on upstream PR jo-inc#5243 (covers bugs 1-2) plus bug 3 which is not
covered by any open PR.
gearwave00001 pushed a commit to gearwave00001/camofox-browser that referenced this pull request Jul 12, 2026
… and launch stability

Combines fixes from two open upstream PRs and adds coverage improvements
neither addresses:

From jo-inc#6788 (mavolty) — server.js:
  - await VirtualDisplay.get() — missing await passed a Promise object as
    the display string, so the browser launched headed with no display
    ("cannot open display") even though Xvfb was running. One-word fix.
  - viewport: null in newContext() — Playwright >=1.58 sends isMobile in
    Browser.setDefaultViewport, which Camoufox v135's Juggler protocol
    rejects. Passing null skips setDefaultViewport entirely; per-page
    setViewportSize() handles sizing.
  - DefaultVirtualDisplay subclass patches Xvfb resolution to 1280x720x24
    so pages still render at the expected size without a context-level
    viewport (which is now null).

From jo-inc#5243 (paranoidi) — vnc-watcher.sh:
  - detect_display() with 3 fallbacks (argv scan, /tmp/.X*-lock files,
    /tmp/.X11-unix/X* sockets) replaces the broken awk regex that only
    matched Xvfb launched with a hardcoded :N display. Camoufox uses
    -displayfd 3 (dynamic assignment), so the old regex never matched
    and x11vnc never started.

Improvements beyond both PRs:
  - Heartbeat check: each loop iteration verifies x11vnc is still alive
    and clears stale PID/display state when it dies. Neither jo-inc#5243 nor
    jo-inc#6788 addresses this — without it, the watcher loses track of a
    crashed x11vnc and never re-attaches, leaving noVNC permanently
    disconnected. This is the bug that made multi-tab sessions unreliable
    (x11vnc dies on browser restart, watcher never recovers).
  - xdpyinfo polling (5s) before starting x11vnc prevents XIO error
    crashes when Xvfb hasn't finished accepting connections.
  - Removed set -e so the watcher survives x11vnc failures instead of
    exiting. Replaced -bg with foreground background (&) plus PID
    verification via $! so failures are visible and stale PIDs aren't
    recorded.
  - killall x11vnc before starting a new instance to avoid port conflicts
    on re-attach.

Also:
  - Add docker-compose.yaml with NODE_OPTIONS=--max-old-space-size=4096
    (fixes JS heap OOM that killed the server on memory-heavy pages),
    ENABLE_VNC=1, and VNC_BIND=0.0.0.0 for remote access.
  - Add launchCompat.test.js to prevent regression on the await and
    viewport: null fixes.
  - Drop executable bit on non-script files (bin/camofox-browser.js,
    release.sh, etc.) that were incorrectly mode 755.

Closes issues from jo-inc#5243, jo-inc#6788, and jo-inc#5916.
Builds on mvanhorn's fixes (jo-inc#5042, jo-inc#5040) for postinstall env whitelist,
ENABLE_VNC=1 honor, and BROWSER_IDLE_TIMEOUT_MS=0 support.
skyfallsin added a commit that referenced this pull request Jul 19, 2026
Incorporates the VNC attachment and recovery work proposed in #4781, #5243, #6549, and #8070 while preserving per-server process ownership.

Co-authored-by: Doud-FR <59610009+Doud-FR@users.noreply.github.com>
Co-authored-by: paranoidi <504877+paranoidi@users.noreply.github.com>
Co-authored-by: Omar Usman <19397228+modanq@users.noreply.github.com>
Co-authored-by: luxles <291718194+luxles@users.noreply.github.com>
@skyfallsin

Copy link
Copy Markdown
Contributor

Thank you — support for Xvfb launched with -displayfd has shipped in v1.12.1. The released implementation resolves the display through the owned Xvfb PID using lock files when available and /proc socket ownership when lock files are absent, avoiding attachment to another Camofox server’s display.

Included via commit 50b5031 with co-author credit.

@skyfallsin skyfallsin closed this Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants