Skip to content

Better server - #3329

Merged
karlseguin merged 5 commits into
mainfrom
better-server
Sep 2, 2026
Merged

karlseguin merged 5 commits into
mainfrom
better-server

Conversation

@karlseguin

Copy link
Copy Markdown
Collaborator

Significant rework of the CDP/BiDi server. There are two main changes:

1 - poll replaced with EPoll/KQueue
2 - make http serving a first class citizen

The change from poll -> epoll/kqueue isn't performance driven, it's just about
tighter code. Both epoll and kqueue let you associate arbitrary data with a
socket, so we don't need to keep arrays in sync in order to associate a socket
with a CDP by index. They both provide some event/notification mechanism, which
is cleaner than the pipe required by poll.

The poll -> epoll/kqueue change could almost have been mechanical. Making HTTP
a first class citizen is the more significant of the two changes

In main, a new connection always spawns a thread and, until does its own
little read loop until the connection is upgraded. This is not efficient, it
uses up a connection slot, and it's inconsistent with the final WebSocket
connection which is polled off the main loop. Using up a slot means that
keepalive isn't possible, else HTTP connections would quickly use up all
available slots/threads.

This commit parses and serves HTTP requests on the main thread (safe
because none of the processing is blocking). The approach is better streamlined
for HTTP requests which never upgrade (/metrics, WebDriver) without causing
any performance overhead for those that do. It simplifies some things (e.g. an
"http" socket or a "websocket" socket is monitored and read in a similar manner
(on the main loop)). It makes other things more complicated; the flow is no
longer accept -> spawn -> upgrade -> websocket loop. It's loop -> accept -> loop
-> process -> (http | ws).

This is built ontop of the BiDi branch because (a) WebDriver is what needs
better HTTP support and (b) some of the more mechanical changes already exist
in that branch (e.g. src/cdp/, src/server.zig -> src/server/*)

Base automatically changed from bidi to main August 31, 2026 15:06
@karlseguin
karlseguin force-pushed the better-server branch 2 times, most recently from 172069b to 81a7534 Compare September 1, 2026 02:06
Comment thread src/server/Link.zig
Comment thread src/server/http.zig
@krichprollsch

krichprollsch commented Sep 1, 2026

Copy link
Copy Markdown
Member

I get a crash on my test:

Lightpanda has crashed. Please report the issue:
https://github.com/lightpanda-io/browser/issues
or let us know on discord: https://discord.gg/g24PtgD6

reason: attempt to use null value
OS: linux
mode: Debug
version: 1.0.0-dev.8994+ebad491fd
source: global
panicked during a panic. Aborting.
run
└─ run exe lightpanda failure
error: process terminated with signal ABRT
failed command: ./.zig-cache/o/62e8c1a5e8e7236a87af96615ce57131/lightpanda

Build Summary: 31/33 steps succeeded (1 failed)
run transitive failure
└─ run exe lightpanda failure

error: the following build command failed with exit code 1:
.zig-cache/o/552d4e306d7293bacfec2178eeaee1bc/build /usr/local/zig-0.16.0/zig /usr/local/zig-0.16.0/lib /home/pierre/wrk/browser-review .zig-cache /home/pierre/.cache/zig --seed 0x69735f8c -Z672f52b44756592d run

here is the script https://gist.github.com/krichprollsch/c9fed136c2bdb467a01563df9452526c
I run it with

LP_WS=ws://127.0.0.1:9222  CONCURRENCY=1 ITERATIONS=100 node lightpanda-repro.mjs

@karlseguin

Copy link
Copy Markdown
Collaborator Author

I get a crash on my test:

version: 1.0.0-dev.8994+ebad491fd

This isn't the latest version of the branch. The bug was in main and fixed in #3337 and was rebased into the branch at some point.

Significant rework of the CDP/BiDi server. There are two main changes:

1 - poll replaced with EPoll/Kqueue (1)
2 - make http serving a first class citizen

The change from poll -> epoll/kqueue isn't performance driven, it's just about
tighter code. Both epoll and kqueue let you associate arbitrary data with a
socket, so we don't need to keep arrays in sync in order to associate a socket
with a CDP by index. They both provide some event/notification mechanism, which
is cleaner than the pipe required by poll.

The poll -> epoll/kqueue change could almost have been mechanical. Making HTTP
a first class citizen is the more significant of the two changes

In `main`, a new connection always spawns a thread and, until does its own
little read loop until the connection is upgraded. This is not efficient, it
uses up a connection slot, and it's inconsistent with the final WebSocket
connection which _is_ polled off the main loop. Using up a slot means that
keepalive isn't possible, else HTTP connections would quickly use up all
available slots/threads.

This commit parses and serves HTTP requests on the main thread (safe
because none of the processing is blocking). The approach is better streamlined
for HTTP requests which never upgrade (/metrics, WebDriver) without causing
any performance overhead for those that do. It simplifies some things (e.g. an
"http" socket or a "websocket" socket is monitored and read in a similar manner
(on the main loop)). It makes other things more complicated; the flow is no
longer accept -> spawn -> upgrade -> websocket loop. It's loop -> accept -> loop
-> process -> (http | ws).

This is built ontop of the BiDi branch because (a) WebDriver is what needs
better  HTTP support and (b) some of the more mechanical changes already exist
in that branch (e.g. src/cdp/, src/server.zig -> src/server/*)

(1) kqueue landing in 2 commits from now on this branch.
Give it one pass through some Claude fuzz testing. Add a max message size,
protect against weird interactions during a shutdown and we had some pending
accepts. Put a time limit on blocked writes.
@karlseguin
karlseguin merged commit 658d350 into main Sep 2, 2026
26 checks passed
@karlseguin
karlseguin deleted the better-server branch September 2, 2026 23:27
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants