Skip to content

web: bound in-flight requests to prevent send-buffer lockup#10615

Merged
gadfort merged 7 commits into
The-OpenROAD-Project:masterfrom
gadfort:web-lockup
Jun 9, 2026
Merged

web: bound in-flight requests to prevent send-buffer lockup#10615
gadfort merged 7 commits into
The-OpenROAD-Project:masterfrom
gadfort:web-lockup

Conversation

@gadfort

@gadfort gadfort commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Rapidly changing selection/zoom enqueued unbounded tile requests,
flooding the WebSocket send buffer until the connection wedged and the
view froze. Add a client-side scheduler that caps concurrent in-flight
requests and queues the rest (cancellable before they hit the wire;
cancelling an already-sent request no longer frees a slot, so churn
can't re-flood). Add liveness detection to force-close and reconnect a
wedged socket, and have the server announce the in-flight limit (scaled
to machine size) on connect. Includes unit tests for the scheduler,
cancellation, liveness, and the announced limit.
Note: claude debugged this and wrote the solution.

Type of Change

  • Bug fix

Impact

Fixes random lockup in the web making it hard to use on large designs.

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@gadfort gadfort requested a review from a team as a code owner June 8, 2026 17:27
@gadfort gadfort requested a review from maliberty June 8, 2026 17:27
@github-actions github-actions Bot added the size/M label Jun 8, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces flow control and liveness monitoring to the WebSocket connection to prevent request flooding and handle wedged connections. On the server side, it sends a configuration payload specifying the maximum in-flight requests based on CPU cores. On the client side, WebSocketManager is updated to queue excess requests, handle cancellation properly, and monitor socket liveness. Feedback on the changes highlights three key robustness improvements: wrapping server-push JSON parsing in a try-catch block to prevent unhandled exceptions, rejecting cancelled queued requests to avoid leaving their promises pending indefinitely, and storing and clearing the liveness monitor interval timer to prevent timer leaks on shutdown.

Comment thread src/web/src/websocket-manager.js Outdated
Comment thread src/web/src/websocket-manager.js Outdated
Comment thread src/web/src/websocket-manager.js
Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@github-actions github-actions Bot added size/L and removed size/M labels Jun 8, 2026
@maliberty

Copy link
Copy Markdown
Member

@codex review

Comment thread src/web/src/web.cpp Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 71583ea4fd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/web/src/websocket-manager.js
Comment thread src/web/src/websocket-manager.js Outdated
Comment thread src/web/src/websocket-manager.js Outdated
@gadfort

gadfort commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@maliberty

Copy link
Copy Markdown
Member

@maliberty not sure what is triggering this: https://github.com/The-OpenROAD-Project/OpenROAD/actions/runs/27206602938/job/80323962147

"arm" is a blocked keyword. I can add an exception or you can pick a different word.

gadfort added 2 commits June 9, 2026 13:00
Signed-off-by: Peter Gadfort <gadfort@zeroasic.com>
@gadfort gadfort enabled auto-merge June 9, 2026 17:20
@gadfort gadfort merged commit 3f91f71 into The-OpenROAD-Project:master Jun 9, 2026
15 of 16 checks passed
@gadfort gadfort deleted the web-lockup branch June 9, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants