From 146985f3ff3d3ec62c656f234874c3caefbf26a4 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Mon, 18 Aug 2025 18:05:44 -0700
Subject: [PATCH] fix: reduce references to Server in codex-login crate (#2398)

Updates the tokio task that monitors `shutdown_notify` and server
requests to ensure that `server.unblock()` is always called, which means
that `ShutdownHandle` only has to invoke `notify_waiters()`.

Now `LoginServer` no longer has to maintain a reference to `Server`. The
`Arc<Server>` only has two active references: the `thread::spawn()` for
reading server messages and the `tokio::task()` that consumes them (and
the shutdown message). Now when shutdown is called (or if login
completes successfully), the `server.unblock()` call ensures the thread
terminates cleanly, which in turn ensures `rx.recv()` in the
`tokio::spawn()` returns `Err`, causing the `tokio::task()` to exit
cleanly, as well.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/2398).
* #2399
* __->__ #2398
* #2396
* #2395
* #2394
* #2393
* #2389
---
 codex-rs/login/src/server.rs | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs
index f3256ee6..e8af09fb 100644
--- a/codex-rs/login/src/server.rs
+++ b/codex-rs/login/src/server.rs
@@ -66,24 +66,14 @@ impl LoginServer {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct ShutdownHandle {
     shutdown_notify: Arc<tokio::sync::Notify>,
-    server: Arc<Server>,
-}
-
-impl std::fmt::Debug for ShutdownHandle {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("ShutdownHandle")
-            .field("shutdown_notify", &self.shutdown_notify)
-            .finish()
-    }
 }
 
 impl ShutdownHandle {
     pub fn shutdown(&self) {
         self.shutdown_notify.notify_waiters();
-        self.server.unblock();
     }
 }
 
@@ -133,14 +123,14 @@ pub fn run_login_server(
         let shutdown_notify = shutdown_notify.clone();
         let server = server.clone();
         tokio::spawn(async move {
-            loop {
+            let result = loop {
                 tokio::select! {
                     _ = shutdown_notify.notified() => {
-                        return Err(io::Error::other("Login was not completed"));
+                        break Err(io::Error::other("Login was not completed"));
                     }
                     maybe_req = rx.recv() => {
                         let Some(req) = maybe_req else {
-                            return Err(io::Error::other("Login was not completed"));
+                            break Err(io::Error::other("Login was not completed"));
                         };
 
                         let url_raw = req.url().to_string();
@@ -159,13 +149,16 @@ pub fn run_login_server(
                         }
 
                         if is_login_complete {
-                            shutdown_notify.notify_waiters();
-                            server.unblock();
-                            return Ok(());
+                            break Ok(());
                         }
                     }
                 }
-            }
+            };
+
+            // Ensure that the server is unblocked so the thread dedicated to
+            // running `server.recv()` in a loop exits cleanly.
+            server.unblock();
+            result
         })
     };
 
@@ -173,10 +166,7 @@ pub fn run_login_server(
         auth_url,
         actual_port,
         server_handle,
-        shutdown_handle: ShutdownHandle {
-            shutdown_notify,
-            server,
-        },
+        shutdown_handle: ShutdownHandle { shutdown_notify },
     })
 }