git.lucas.co / cce-browser
web browser (Servo)
git clone https://git.lucas.co/cce-browser.git

commit3ffb9d43eaedb45a0c6efc2acec3ddab016f96cf
parentc03422bbe5
authorLucas Galante <lsgalante12@gmail.com>
date2026-10-02 13:18
refactor(instance): single-instance on cce_ui::ipc::instance

The claim, the connect-before-bind race, the parked listener and the
bounded reads moved to cce-ui (40c66d4); cce-notes and cce-graph had
copied them from here. instance.rs keeps the browser's protocol: the
launch line (a relative path travels absolute) and its parse.

cce-browser-open mirrors the client half and now bounds its wait for the
ack at 5s like the shared one, so a stuck instance cannot hang a click.

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

 src/bin/open.rs |  14 ++---
 src/instance.rs | 162 ++++++++++++--------------------------------------------
 2 files changed, 43 insertions(+), 133 deletions(-)

diff --git a/src/bin/open.rs b/src/bin/open.rs
index 6180f8e..8498210 100644
--- a/src/bin/open.rs
+++ b/src/bin/open.rs
@@ -7,9 +7,9 @@
 //! `exec` the real browser when no instance answers.
 //!
 //! It therefore deliberately duplicates the client half of the socket
-//! protocol instead of importing it: `src/instance.rs` (same crate) is the
-//! server side and the fallback client, `cce_ui::ipc::socket_path` is the
-//! path convention. All three must agree on `/tmp/cce-browser-<display>.sock`
+//! protocol instead of importing it: `src/instance.rs` (same crate, on
+//! `cce_ui::ipc::instance`) is the server side and the fallback client,
+//! `cce_ui::ipc::socket_path` is the path convention. All three must agree on `/tmp/cce-browser-<display>.sock`
 //! and the `open <arg>` / `new-tab` lines. The protocol is small on purpose;
 //! change it in both files or not at all.
 
@@ -26,9 +26,10 @@ fn socket_path() -> String {
     }
 }
 
-/// One forwarding attempt; false on any failure. Mirrors
-/// `instance::try_forward`, ack wait included — exiting on write alone races
-/// the instance actually reading the line.
+/// One forwarding attempt; false on any failure. Mirrors the client half of
+/// `cce_ui::ipc::instance::forward_or_claim`, ack wait included — exiting on
+/// write alone races the instance actually reading the line — and so is its
+/// bound on that wait: a stuck instance must not hang every click forever.
 fn try_forward(arg: Option<&str>) -> bool {
     let Ok(mut stream) = UnixStream::connect(socket_path()) else {
         return false;
@@ -52,6 +53,7 @@ fn try_forward(arg: Option<&str>) -> bool {
     if stream.write_all(command.as_bytes()).is_err() {
         return false;
     }
+    let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5)));
     let mut reply = String::new();
     BufReader::new(stream).read_line(&mut reply).is_ok()
 }
diff --git a/src/instance.rs b/src/instance.rs
index 8f09c39..dc7604f 100644
--- a/src/instance.rs
+++ b/src/instance.rs
@@ -8,31 +8,16 @@
 //! shadow sessions isolated for free), and every later launch hands its
 //! argument to it and exits before any Wayland or engine work happens.
 //!
-//! The order in [`forward_or_claim`] is what closes the startup race: try to
-//! connect, and only bind after a connect has failed. A refused connection
-//! means the socket file outlived a crashed instance and is removed before
-//! binding; losing the bind to a simultaneous launch falls back to one more
-//! connect. If that also fails the launch proceeds un-listened rather than
-//! not at all.
-//!
-//! The claimed listener has to survive from `main()` (before the engine
-//! starts) to `BrowserApp::new` (where the calloop sender first exists), so
-//! it parks in a static until [`spawn_listener`] adopts it.
-
-use std::io::{BufRead, BufReader, Write};
-use std::os::unix::net::{UnixListener, UnixStream};
-use std::sync::Mutex;
+//! The claim, the race it closes, the parked listener and the bounded reads
+//! are `cce_ui::ipc::instance`'s; this module is the browser's protocol on
+//! top: `open <arg>` / `new-tab`, each answered `ok`. `src/bin/open.rs`
+//! speaks the same protocol without linking cce-ui — keep the two in step.
 
 use crate::Message;
 
 /// Socket prefix; `cce_ui::ipc::socket_path` appends `-<WAYLAND_DISPLAY>`.
 const PREFIX: &str = "cce-browser";
 
-/// The listener claimed by `forward_or_claim`, waiting for `spawn_listener`.
-static CLAIMED: Mutex<Option<UnixListener>> = Mutex::new(None);
-/// The socket path this process bound (and must unlink on exit), if any.
-static OWNED_PATH: Mutex<Option<String>> = Mutex::new(None);
-
 /// Hand `arg` to a running instance, or claim the instance socket.
 ///
 /// Returns `true` when a running instance took the launch (the caller should
@@ -41,83 +26,31 @@ static OWNED_PATH: Mutex<Option<String>> = Mutex::new(None);
 /// single-instance handling failed entirely and the launch should proceed
 /// standalone.
 pub fn forward_or_claim(arg: Option<&str>) -> bool {
-    let path = cce_ui::ipc::socket_path(PREFIX);
-
-    if try_forward(&path, arg) {
-        return true;
-    }
-
-    // Nothing answered. A socket file that still exists is a leftover from a
-    // crashed instance; binding needs it gone.
-    if std::path::Path::new(&path).exists() {
-        let _ = std::fs::remove_file(&path);
-    }
-    match UnixListener::bind(&path) {
-        Ok(listener) => {
-            *CLAIMED.lock().unwrap() = Some(listener);
-            *OWNED_PATH.lock().unwrap() = Some(path);
-            false
-        }
-        // Lost the bind race to a simultaneous launch: it is the instance.
-        Err(_) => try_forward(&path, arg),
-    }
+    cce_ui::ipc::instance::forward_or_claim(PREFIX, &launch_line(arg))
 }
 
-/// One forwarding attempt. False on any failure — there is no retry inside.
-fn try_forward(path: &str, arg: Option<&str>) -> bool {
-    let Ok(mut stream) = UnixStream::connect(path) else {
-        return false;
+/// The line a launch sends. A relative file path is resolved against *this*
+/// process's cwd — the instance's differs, so it must travel absolute.
+fn launch_line(arg: Option<&str>) -> String {
+    let Some(a) = arg else {
+        return "new-tab".to_string();
     };
-    // A relative file path is resolved against *this* process's cwd — the
-    // instance's differs, so it must travel absolute.
-    let command = match arg {
-        Some(a) => {
-            let p = std::path::Path::new(a);
-            let abs = if p.exists() {
-                std::fs::canonicalize(p)
-                    .ok()
-                    .and_then(|c| c.to_str().map(String::from))
-            } else {
-                None
-            };
-            format!("open {}\n", abs.as_deref().unwrap_or(a))
-        }
-        None => "new-tab\n".to_string(),
+    let p = std::path::Path::new(a);
+    let abs = if p.exists() {
+        std::fs::canonicalize(p).ok().and_then(|c| c.to_str().map(String::from))
+    } else {
+        None
     };
-    if stream.write_all(command.as_bytes()).is_err() {
-        return false;
-    }
-    // Wait for the ack: returning (and exiting) on write alone races the
-    // instance actually reading the line.
-    // Bounded: an instance whose listener is stuck must not hang this
-    // launch forever; unanswered, the launch is not forwarded.
-    let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(5)));
-    let mut reply = String::new();
-    BufReader::new(stream).read_line(&mut reply).is_ok()
+    format!("open {}", abs.as_deref().unwrap_or(a))
 }
 
-/// Adopt the listener claimed in `main()` and serve it on a thread, pushing
-/// each received launch into the app's calloop channel. No-op when this
-/// process runs standalone.
+/// Serve the listener claimed in `main()`, pushing each received launch into
+/// the app's calloop channel. No-op when this process runs standalone.
 pub fn spawn_listener(sender: calloop::channel::Sender<Message>) {
-    let Some(listener) = CLAIMED.lock().unwrap().take() else {
-        return;
-    };
-    std::thread::spawn(move || {
-        for conn in listener.incoming() {
-            let Ok(conn) = conn else { continue };
-            let Some(line) = read_request_line(&conn, 64 * 1024, std::time::Duration::from_secs(2)) else {
-                continue;
-            };
-            let msg = match parse_command(line.trim()) {
-                Some(m) => m,
-                None => continue,
-            };
-            if sender.send(msg).is_err() {
-                return; // channel gone: the app is shutting down
-            }
-            let _ = (&conn).write_all(b"ok\n");
-        }
+    cce_ui::ipc::instance::serve(move |line| {
+        let msg = parse_command(line)?;
+        sender.send(msg).ok()?;
+        Some("ok".into())
     });
 }
 
@@ -134,12 +67,9 @@ fn parse_command(line: &str) -> Option<Message> {
 }
 
 /// Unlink the socket if this process bound it. Called after the engine loop
-/// returns; a crash skips it, which is what the stale-socket removal in
-/// [`forward_or_claim`] exists for.
+/// returns.
 pub fn cleanup() {
-    if let Some(path) = OWNED_PATH.lock().unwrap().take() {
-        let _ = std::fs::remove_file(path);
-    }
+    cce_ui::ipc::instance::cleanup();
 }
 
 #[cfg(test)]
@@ -159,39 +89,17 @@ mod tests {
         assert!(parse_command("open ").is_none());
         assert!(parse_command("bogus").is_none());
     }
-}
 
-/// One request line from a control-socket client, bounded in size and in
-/// TOTAL time. Until 2026-10-02 this was `BufReader::read_line` on a socket
-/// with no timeout at all, so a client that connected and said nothing (or trickled a
-/// byte at a time) held the listener thread for good, and every later
-/// launch forwarded to it (a link opened from another app) waited behind. None on EOF before any byte, timeout,
-/// overflow or a read error.
-fn read_request_line(conn: &std::os::unix::net::UnixStream, limit: usize, deadline: std::time::Duration) -> Option<String> {
-    use std::io::Read;
-    let until = std::time::Instant::now() + deadline;
-    let mut buf: Vec<u8> = Vec::new();
-    let mut chunk = [0u8; 4096];
-    let mut reader = conn;
-    loop {
-        let left = until.checked_duration_since(std::time::Instant::now()).filter(|d| !d.is_zero())?;
-        conn.set_read_timeout(Some(left)).ok()?;
-        let n = reader.read(&mut chunk).ok()?;
-        if n == 0 {
-            if buf.is_empty() {
-                return None;
-            }
-            break;
-        }
-        buf.extend_from_slice(&chunk[..n]);
-        if let Some(end) = buf.iter().position(|&b| b == b'\n') {
-            buf.truncate(end + 1);
-            break;
-        }
-        if buf.len() > limit {
-            return None;
-        }
+    #[test]
+    fn launch_lines_round_trip() {
+        assert_eq!(launch_line(None), "new-tab");
+        assert_eq!(launch_line(Some("https://example.com")), "open https://example.com");
+        assert!(matches!(
+            parse_command(&launch_line(Some("https://example.com"))),
+            Some(Message::OpenExternal(Some(u))) if u == "https://example.com"
+        ));
+        // A path that exists here travels absolute.
+        let line = launch_line(Some("Cargo.toml"));
+        assert!(line.starts_with("open /") && line.ends_with("/Cargo.toml"), "{line}");
     }
-    let _ = conn.set_read_timeout(None);
-    String::from_utf8(buf).ok()
 }