web: address feedback

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>

Author: gadfort

src/web/include/web/web.h

@@ -60,7 +60,8 @@ ListenerHandle createAndRunListener(
     std::shared_ptr<TimingReport> timing_report,
     std::shared_ptr<ClockTreeReport> clock_report,
     utl::Logger* logger,
-    WebViewerHook* viewer_hook);
+    WebViewerHook* viewer_hook,
+    int max_in_flight);
 
 // A layout web server.  serve() starts the server in background I/O
 // threads; waitForStop() blocks the calling thread until requestStop()

src/web/src/web.cpp

@@ -157,14 +157,18 @@ class WebSocketSession : public std::enable_shared_from_this<WebSocketSession>,
   WebViewerHook* viewer_hook_ = nullptr;
   std::size_t viewer_token_ = 0;
 
+  // In-flight request window announced to the client on connect.
+  int max_in_flight_ = 16;
+
  public:
   WebSocketSession(Tcp::socket&& socket,
                    std::shared_ptr<TileGenerator> generator,
                    std::shared_ptr<TclEvaluator> tcl_eval,
                    std::shared_ptr<TimingReport> timing_report,
                    std::shared_ptr<ClockTreeReport> clock_report,
                    utl::Logger* logger,
-                   WebViewerHook* viewer_hook);
+                   WebViewerHook* viewer_hook,
+                   int max_in_flight);
   ~WebSocketSession();
 
   void run(http::request<http::string_body>&& req);
@@ -223,7 +227,8 @@ WebSocketSession::WebSocketSession(
     std::shared_ptr<TimingReport> timing_report,
     std::shared_ptr<ClockTreeReport> clock_report,
     utl::Logger* logger,
-    WebViewerHook* viewer_hook)
+    WebViewerHook* viewer_hook,
+    int max_in_flight)
     : websocket_(std::move(socket)),
       logger_(logger),
       select_handler_(generator, tcl_eval),
@@ -234,7 +239,8 @@ WebSocketSession::WebSocketSession(
       drc_handler_(generator),
       strand_(net::make_strand(websocket_.get_executor())),
       generator_(std::move(generator)),
-      viewer_hook_(viewer_hook)
+      viewer_hook_(viewer_hook),
+      max_in_flight_(max_in_flight)
 {
   if (generator_) {
     odb::dbChip* chip = generator_->getChip();
@@ -433,25 +439,15 @@ void WebSocketSession::on_accept(beast::error_code ec)
 
   // Tell the client how many requests to keep in flight at once. This bounds
   // the client's send rate so a burst of tile requests (rapid pan/zoom) can't
-  // flood the socket send buffer and wedge the connection. Scale with the
-  // machine size — a larger box can absorb more concurrency — and clamp to a
-  // sane range. Sent first so the client has it before requesting any tiles.
+  // flood the socket send buffer and wedge the connection. The limit is scaled
+  // to the server's I/O worker count (see WebServer::serve). Sent first so the
+  // client has it before requesting any tiles.
   {
-    unsigned int cores = std::thread::hardware_concurrency();
-    if (cores == 0) {
-      cores = 4;  // hardware_concurrency may report 0; pick a modest default
-    }
-    int max_in_flight = static_cast<int>(cores) * 4;
-    if (max_in_flight < 16) {
-      max_in_flight = 16;
-    } else if (max_in_flight > 256) {
-      max_in_flight = 256;
-    }
     WebSocketResponse cfg;
     cfg.id = 0;
     cfg.type = WebSocketResponse::kJson;
     const std::string cfg_json = R"({"type":"config","max_in_flight":)"
-                                 + std::to_string(max_in_flight) + "}";
+                                 + std::to_string(max_in_flight_) + "}";
     cfg.payload.assign(cfg_json.begin(), cfg_json.end());
     queue_response(cfg);
   }
@@ -749,6 +745,7 @@ class DetectSession : public std::enable_shared_from_this<DetectSession>
   http::request<http::string_body> req_;
   utl::Logger* logger_;
   WebViewerHook* viewer_hook_ = nullptr;
+  int max_in_flight_ = 16;
 
  public:
   DetectSession(Tcp::socket&& socket,
@@ -757,7 +754,8 @@ class DetectSession : public std::enable_shared_from_this<DetectSession>
                 std::shared_ptr<TimingReport> timing_report,
                 std::shared_ptr<ClockTreeReport> clock_report,
                 utl::Logger* logger,
-                WebViewerHook* viewer_hook);
+                WebViewerHook* viewer_hook,
+                int max_in_flight);
 
   void run();
 
@@ -771,14 +769,16 @@ DetectSession::DetectSession(Tcp::socket&& socket,
                              std::shared_ptr<TimingReport> timing_report,
                              std::shared_ptr<ClockTreeReport> clock_report,
                              utl::Logger* logger,
-                             WebViewerHook* viewer_hook)
+                             WebViewerHook* viewer_hook,
+                             int max_in_flight)
     : stream_(std::move(socket)),
       generator_(std::move(generator)),
       tcl_eval_(std::move(tcl_eval)),
       timing_report_(std::move(timing_report)),
       clock_report_(std::move(clock_report)),
       logger_(logger),
-      viewer_hook_(viewer_hook)
+      viewer_hook_(viewer_hook),
+      max_in_flight_(max_in_flight)
 {
 }
 
@@ -810,7 +810,8 @@ void DetectSession::on_read(beast::error_code ec)
                                              timing_report_,
                                              clock_report_,
                                              logger_,
-                                             viewer_hook_);
+                                             viewer_hook_,
+                                             max_in_flight_);
     websocket_session->run(std::move(req_));
   } else {
     // Regular HTTP - hand off to session with already-read request
@@ -833,6 +834,7 @@ class Listener : public std::enable_shared_from_this<Listener>
   std::shared_ptr<ClockTreeReport> clock_report_;
   utl::Logger* logger_;
   WebViewerHook* viewer_hook_ = nullptr;
+  int max_in_flight_ = 16;
 
  public:
   Listener(net::io_context& ioc,
@@ -842,7 +844,8 @@ class Listener : public std::enable_shared_from_this<Listener>
            std::shared_ptr<TimingReport> timing_report,
            std::shared_ptr<ClockTreeReport> clock_report,
            utl::Logger* logger,
-           WebViewerHook* viewer_hook);
+           WebViewerHook* viewer_hook,
+           int max_in_flight);
 
   void run() { do_accept(); }
 
@@ -870,15 +873,17 @@ Listener::Listener(net::io_context& ioc,
                    std::shared_ptr<TimingReport> timing_report,
                    std::shared_ptr<ClockTreeReport> clock_report,
                    utl::Logger* logger,
-                   WebViewerHook* viewer_hook)
+                   WebViewerHook* viewer_hook,
+                   int max_in_flight)
     : ioc_(ioc),
       acceptor_(ioc),
       generator_(std::move(generator)),
       tcl_eval_(std::move(tcl_eval)),
       timing_report_(std::move(timing_report)),
       clock_report_(std::move(clock_report)),
       logger_(logger),
-      viewer_hook_(viewer_hook)
+      viewer_hook_(viewer_hook),
+      max_in_flight_(max_in_flight)
 {
   beast::error_code ec;
 
@@ -929,7 +934,8 @@ void Listener::on_accept(beast::error_code ec, Tcp::socket socket)
                                     timing_report_,
                                     clock_report_,
                                     logger_,
-                                    viewer_hook_)
+                                    viewer_hook_,
+                                    max_in_flight_)
         ->run();
   }
   do_accept();
@@ -1317,7 +1323,8 @@ ListenerHandle createAndRunListener(
     std::shared_ptr<TimingReport> timing_report,
     std::shared_ptr<ClockTreeReport> clock_report,
     utl::Logger* logger,
-    WebViewerHook* viewer_hook)
+    WebViewerHook* viewer_hook,
+    int max_in_flight)
 {
   auto listener = std::make_shared<Listener>(ioc,
                                              endpoint,
@@ -1326,7 +1333,8 @@ ListenerHandle createAndRunListener(
                                              std::move(timing_report),
                                              std::move(clock_report),
                                              logger,
-                                             viewer_hook);
+                                             viewer_hook,
+                                             max_in_flight);
   listener->run();
   return {.shutdown = [listener]() { listener->close(); },
           .port = listener->port()};

src/web/src/web_serve.cpp

@@ -6,6 +6,7 @@
 // references (which would require the full gui library including Qt
 // SWIG wrappers and ord::OpenRoad symbols).
 
+#include <algorithm>
 #include <chrono>
 #include <cstdint>
 #include <cstdlib>
@@ -241,6 +242,13 @@ void WebServer::serve(int port)
     uint16_t const u_port = port;
     int const num_threads = num_threads_;
 
+    // Bound how many requests the client keeps in flight at once. Scale with
+    // the server's I/O worker count (the threads that actually service
+    // requests) so the window tracks the configured thread budget, with an
+    // absolute cap so a many-core box doesn't hand out an unbounded window.
+    // Announced to the client on connect; see WebSocketSession::on_accept.
+    int const max_in_flight = std::clamp(num_threads * 4, 16, 256);
+
     ioc_ = std::make_unique<net::io_context>(num_threads);
 
     auto handle = createAndRunListener(*ioc_,
@@ -250,7 +258,8 @@ void WebServer::serve(int port)
                                        timing_report,
                                        clock_report,
                                        logger_,
-                                       viewer_hook_.get());
+                                       viewer_hook_.get(),
+                                       max_in_flight);
     shutdown_listener_ = std::move(handle.shutdown);
 
     const std::string url = "http://localhost:" + std::to_string(handle.port);

src/web/src/websocket-manager.js

@@ -33,11 +33,16 @@ const MAX_BUFFERED_BYTES = 512 * 1024;
 //   2. We are saturated (in-flight at the cap) with work still queued, yet have
 //      heard nothing back for DEAD_MS. A slow render is one slow reply among
 //      fast ones, so replies keep flowing; total silence while saturated and
-//      backed up means the server stopped servicing us entirely.
+//      backed up means the server stopped servicing us entirely. This only ever
+//      arms during a request flood (the window is full with more queued) — a
+//      lone slow command (e.g. a timing query that triggers an STA update)
+//      never saturates the window, so it cannot trip this. DEAD_MS is also set
+//      well beyond any plausible single render so a saturated-but-slow burst
+//      can't be mistaken for a dead server.
 // On either, we force-close the socket so onclose reconnects and recovers.
 const STUCK_BYTES = 256 * 1024;
 const STUCK_MS = 12000;
-const DEAD_MS = 15000;
+const DEAD_MS = 30000;
 const LIVENESS_INTERVAL_MS = 3000;
 
 export class WebSocketManager {
@@ -128,36 +133,64 @@ export class WebSocketManager {
             this.handleMessage(event.data);
         };
 
-        this.socket.onclose = () => {
-            this._isConnected = false;
-            this._inFlight = 0;
-            this._bufStuckSince = 0;
-            this.onStatusChange();
-            for (const [id, handler] of this.pending) {
-                handler.reject(new Error('WebSocket closed'));
-            }
-            this.pending.clear();
-            // Drop anything still queued — it never reached the wire. Reject so
-            // callers settle; tile callers .catch() and re-request on reconnect.
-            for (const [id, entry] of this._queue) {
-                entry.reject(new Error('WebSocket closed'));
-            }
-            this._queue.clear();
-            if (this._shutdown) {
-                console.log('WebSocket closed (server stopped)');
-                this._stopLivenessMonitor(); // no socket will reopen
-                return; // don't reconnect after intentional shutdown
-            }
-            console.log('WebSocket closed, reconnecting...');
-            setTimeout(() => this.connect(), this.reconnectDelay);
-            this.reconnectDelay = Math.min(this.reconnectDelay * 2, 30000);
-        };
+        this.socket.onclose = () => this._handleClose();
 
         this.socket.onerror = (err) => {
             console.error('WebSocket error:', err);
         };
     }
 
+    // Tear down after the socket is gone (rejecting outstanding work) and
+    // schedule a reconnect unless the server told us to shut down. Invoked by
+    // socket.onclose and, for a wedged socket, directly by _forceReconnect().
+    _handleClose() {
+        this._isConnected = false;
+        this._inFlight = 0;
+        this._bufStuckSince = 0;
+        this.onStatusChange();
+        for (const [id, handler] of this.pending) {
+            handler.reject(new Error('WebSocket closed'));
+        }
+        this.pending.clear();
+        // Drop anything still queued — it never reached the wire. Reject so
+        // callers settle; tile callers .catch() and re-request on reconnect.
+        for (const [id, entry] of this._queue) {
+            entry.reject(new Error('WebSocket closed'));
+        }
+        this._queue.clear();
+        if (this._shutdown) {
+            console.log('WebSocket closed (server stopped)');
+            this._stopLivenessMonitor(); // no socket will reopen
+            return; // don't reconnect after intentional shutdown
+        }
+        console.log('WebSocket closed, reconnecting...');
+        setTimeout(() => this.connect(), this.reconnectDelay);
+        this.reconnectDelay = Math.min(this.reconnectDelay * 2, 30000);
+    }
+
+    // Abandon a wedged socket and reconnect now, without waiting for close()
+    // to fire onclose. When the send buffer is stuck, close() only starts its
+    // handshake after the already-buffered bytes drain (which they aren't), so
+    // onclose may not arrive until the TCP timeout — minutes away. Detach the
+    // dead socket's handlers so it can no longer drive our state, run the same
+    // teardown onclose would, then let _handleClose() schedule the reconnect.
+    _forceReconnect() {
+        const dead = this.socket;
+        this.socket = null;
+        if (dead) {
+            dead.onopen = null;
+            dead.onmessage = null;
+            dead.onclose = null;
+            dead.onerror = null;
+            try {
+                dead.close(); // best effort; we no longer depend on it
+            } catch (e) {
+                /* already gone */
+            }
+        }
+        this._handleClose();
+    }
+
     handleMessage(data) {
         // Binary frame: [4B id][1B type][3B reserved][payload...]
         const view = new DataView(data);
@@ -359,13 +392,11 @@ export class WebSocketManager {
 
         if (wedged) {
             console.warn(`WebSocket connection wedged — ${why}; `
-                + `closing to recover`);
+                + `reconnecting to recover`);
             this._bufStuckSince = 0;
-            try {
-                s.close(); // onclose clears state, rejects pending, reconnects
-            } catch (e) {
-                /* onclose still fires */
-            }
+            // Don't depend on close()/onclose: a stuck send buffer can defer
+            // the close handshake indefinitely. Reconnect immediately.
+            this._forceReconnect();
         }
     }