Skip to content

Commit 3f91f71

Browse files
authored
Merge pull request #10615 from gadfort/web-lockup
web: bound in-flight requests to prevent send-buffer lockup
2 parents 8df74aa + c3e843d commit 3f91f71

6 files changed

Lines changed: 638 additions & 43 deletions

File tree

src/web/include/web/web.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ ListenerHandle createAndRunListener(
6060
std::shared_ptr<TimingReport> timing_report,
6161
std::shared_ptr<ClockTreeReport> clock_report,
6262
utl::Logger* logger,
63-
WebViewerHook* viewer_hook);
63+
WebViewerHook* viewer_hook,
64+
int max_in_flight);
6465

6566
// A layout web server. serve() starts the server in background I/O
6667
// threads; waitForStop() blocks the calling thread until requestStop()

src/web/src/main.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const DISCONNECT_DELAY_MS = 2000; // Show banner after 2 seconds of disconnectio
2929

3030
function updateStatus() {
3131
const isConnected = app.websocketManager && app.websocketManager.isConnected;
32-
const pendingCount = app.websocketManager ? app.websocketManager.pending.size : 0;
32+
const pendingCount = app.websocketManager ? app.websocketManager.pendingCount : 0;
3333

3434
if (!isConnected) {
3535
// After an intentional shutdown the "Server stopped" banner is

src/web/src/web.cpp

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,18 @@ class WebSocketSession : public std::enable_shared_from_this<WebSocketSession>,
157157
WebViewerHook* viewer_hook_ = nullptr;
158158
std::size_t viewer_token_ = 0;
159159

160+
// In-flight request window announced to the client on connect.
161+
int max_in_flight_ = 16;
162+
160163
public:
161164
WebSocketSession(Tcp::socket&& socket,
162165
std::shared_ptr<TileGenerator> generator,
163166
std::shared_ptr<TclEvaluator> tcl_eval,
164167
std::shared_ptr<TimingReport> timing_report,
165168
std::shared_ptr<ClockTreeReport> clock_report,
166169
utl::Logger* logger,
167-
WebViewerHook* viewer_hook);
170+
WebViewerHook* viewer_hook,
171+
int max_in_flight);
168172
~WebSocketSession();
169173

170174
void run(http::request<http::string_body>&& req);
@@ -223,7 +227,8 @@ WebSocketSession::WebSocketSession(
223227
std::shared_ptr<TimingReport> timing_report,
224228
std::shared_ptr<ClockTreeReport> clock_report,
225229
utl::Logger* logger,
226-
WebViewerHook* viewer_hook)
230+
WebViewerHook* viewer_hook,
231+
int max_in_flight)
227232
: websocket_(std::move(socket)),
228233
logger_(logger),
229234
select_handler_(generator, tcl_eval),
@@ -234,7 +239,8 @@ WebSocketSession::WebSocketSession(
234239
drc_handler_(generator),
235240
strand_(net::make_strand(websocket_.get_executor())),
236241
generator_(std::move(generator)),
237-
viewer_hook_(viewer_hook)
242+
viewer_hook_(viewer_hook),
243+
max_in_flight_(max_in_flight)
238244
{
239245
if (generator_) {
240246
odb::dbChip* chip = generator_->getChip();
@@ -431,6 +437,21 @@ void WebSocketSession::on_accept(beast::error_code ec)
431437
viewer_hook_->drainLogs();
432438
}
433439

440+
// Tell the client how many requests to keep in flight at once. This bounds
441+
// the client's send rate so a burst of tile requests (rapid pan/zoom) can't
442+
// flood the socket send buffer and wedge the connection. The limit is scaled
443+
// to the server's I/O worker count (see WebServer::serve). Sent first so the
444+
// client has it before requesting any tiles.
445+
{
446+
WebSocketResponse cfg;
447+
cfg.id = 0;
448+
cfg.type = WebSocketResponse::kJson;
449+
const std::string cfg_json = R"({"type":"config","max_in_flight":)"
450+
+ std::to_string(max_in_flight_) + "}";
451+
cfg.payload.assign(cfg_json.begin(), cfg_json.end());
452+
queue_response(cfg);
453+
}
454+
434455
// Build search indices in the background; tiles render without shapes
435456
// until ready, then a "refresh" push notification triggers a redraw.
436457
init_thread_ = std::thread([self = shared_from_this()]() {
@@ -724,6 +745,7 @@ class DetectSession : public std::enable_shared_from_this<DetectSession>
724745
http::request<http::string_body> req_;
725746
utl::Logger* logger_;
726747
WebViewerHook* viewer_hook_ = nullptr;
748+
int max_in_flight_ = 16;
727749

728750
public:
729751
DetectSession(Tcp::socket&& socket,
@@ -732,7 +754,8 @@ class DetectSession : public std::enable_shared_from_this<DetectSession>
732754
std::shared_ptr<TimingReport> timing_report,
733755
std::shared_ptr<ClockTreeReport> clock_report,
734756
utl::Logger* logger,
735-
WebViewerHook* viewer_hook);
757+
WebViewerHook* viewer_hook,
758+
int max_in_flight);
736759

737760
void run();
738761

@@ -746,14 +769,16 @@ DetectSession::DetectSession(Tcp::socket&& socket,
746769
std::shared_ptr<TimingReport> timing_report,
747770
std::shared_ptr<ClockTreeReport> clock_report,
748771
utl::Logger* logger,
749-
WebViewerHook* viewer_hook)
772+
WebViewerHook* viewer_hook,
773+
int max_in_flight)
750774
: stream_(std::move(socket)),
751775
generator_(std::move(generator)),
752776
tcl_eval_(std::move(tcl_eval)),
753777
timing_report_(std::move(timing_report)),
754778
clock_report_(std::move(clock_report)),
755779
logger_(logger),
756-
viewer_hook_(viewer_hook)
780+
viewer_hook_(viewer_hook),
781+
max_in_flight_(max_in_flight)
757782
{
758783
}
759784

@@ -785,7 +810,8 @@ void DetectSession::on_read(beast::error_code ec)
785810
timing_report_,
786811
clock_report_,
787812
logger_,
788-
viewer_hook_);
813+
viewer_hook_,
814+
max_in_flight_);
789815
websocket_session->run(std::move(req_));
790816
} else {
791817
// Regular HTTP - hand off to session with already-read request
@@ -808,6 +834,7 @@ class Listener : public std::enable_shared_from_this<Listener>
808834
std::shared_ptr<ClockTreeReport> clock_report_;
809835
utl::Logger* logger_;
810836
WebViewerHook* viewer_hook_ = nullptr;
837+
int max_in_flight_ = 16;
811838

812839
public:
813840
Listener(net::io_context& ioc,
@@ -817,7 +844,8 @@ class Listener : public std::enable_shared_from_this<Listener>
817844
std::shared_ptr<TimingReport> timing_report,
818845
std::shared_ptr<ClockTreeReport> clock_report,
819846
utl::Logger* logger,
820-
WebViewerHook* viewer_hook);
847+
WebViewerHook* viewer_hook,
848+
int max_in_flight);
821849

822850
void run() { do_accept(); }
823851

@@ -845,15 +873,17 @@ Listener::Listener(net::io_context& ioc,
845873
std::shared_ptr<TimingReport> timing_report,
846874
std::shared_ptr<ClockTreeReport> clock_report,
847875
utl::Logger* logger,
848-
WebViewerHook* viewer_hook)
876+
WebViewerHook* viewer_hook,
877+
int max_in_flight)
849878
: ioc_(ioc),
850879
acceptor_(ioc),
851880
generator_(std::move(generator)),
852881
tcl_eval_(std::move(tcl_eval)),
853882
timing_report_(std::move(timing_report)),
854883
clock_report_(std::move(clock_report)),
855884
logger_(logger),
856-
viewer_hook_(viewer_hook)
885+
viewer_hook_(viewer_hook),
886+
max_in_flight_(max_in_flight)
857887
{
858888
beast::error_code ec;
859889

@@ -904,7 +934,8 @@ void Listener::on_accept(beast::error_code ec, Tcp::socket socket)
904934
timing_report_,
905935
clock_report_,
906936
logger_,
907-
viewer_hook_)
937+
viewer_hook_,
938+
max_in_flight_)
908939
->run();
909940
}
910941
do_accept();
@@ -1292,7 +1323,8 @@ ListenerHandle createAndRunListener(
12921323
std::shared_ptr<TimingReport> timing_report,
12931324
std::shared_ptr<ClockTreeReport> clock_report,
12941325
utl::Logger* logger,
1295-
WebViewerHook* viewer_hook)
1326+
WebViewerHook* viewer_hook,
1327+
int max_in_flight)
12961328
{
12971329
auto listener = std::make_shared<Listener>(ioc,
12981330
endpoint,
@@ -1301,7 +1333,8 @@ ListenerHandle createAndRunListener(
13011333
std::move(timing_report),
13021334
std::move(clock_report),
13031335
logger,
1304-
viewer_hook);
1336+
viewer_hook,
1337+
max_in_flight);
13051338
listener->run();
13061339
return {.shutdown = [listener]() { listener->close(); },
13071340
.port = listener->port()};

src/web/src/web_serve.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// references (which would require the full gui library including Qt
77
// SWIG wrappers and ord::OpenRoad symbols).
88

9+
#include <algorithm>
910
#include <chrono>
1011
#include <cstdint>
1112
#include <cstdlib>
@@ -241,6 +242,13 @@ void WebServer::serve(int port)
241242
uint16_t const u_port = port;
242243
int const num_threads = num_threads_;
243244

245+
// Bound how many requests the client keeps in flight at once. Scale with
246+
// the server's I/O worker count (the threads that actually service
247+
// requests) so the window tracks the configured thread budget, with an
248+
// absolute cap so a many-core box doesn't hand out an unbounded window.
249+
// Announced to the client on connect; see WebSocketSession::on_accept.
250+
int const max_in_flight = std::clamp(num_threads * 4, 16, 256);
251+
244252
ioc_ = std::make_unique<net::io_context>(num_threads);
245253

246254
auto handle = createAndRunListener(*ioc_,
@@ -250,7 +258,8 @@ void WebServer::serve(int port)
250258
timing_report,
251259
clock_report,
252260
logger_,
253-
viewer_hook_.get());
261+
viewer_hook_.get(),
262+
max_in_flight);
254263
shutdown_listener_ = std::move(handle.shutdown);
255264

256265
const std::string url = "http://localhost:" + std::to_string(handle.port);

0 commit comments

Comments
 (0)