Skip to content

Close a WebSocket client after a failed or partial frame write - #728

Merged
AlbertShown merged 1 commit into
webui-dev:mainfrom
ViktorJannicke:fix/ws-partial-write
Oct 26, 2026
Merged

AlbertShown merged 1 commit into
webui-dev:mainfrom
ViktorJannicke:fix/ws-partial-write

Conversation

@ViktorJannicke

Copy link
Copy Markdown
Contributor

Problem

_webui_send_client_ws() stores the result of mg_websocket_write() in ret and only logs it (src/webui.c:10897 on 2fb97ed). CivetWeb's mg_websocket_write_exec() writes the frame header and then the payload with two mg_write() calls (src/civetweb/civetweb.c:13950, :13962). It returns -1 if the header was not written completely, otherwise the result of the payload write, which can be fewer than packets_size bytes or -1. A write ends early when the client stops reading for longer than request_timeout_ms (default 30 s, push_all(), civetweb.c:6327; WebUI doesn't set it) or when the socket fails.

Part of a frame is then on the wire, but the connection stays open and the next frames are written after it. The client reads them out of step: the rest of the cut frame's payload is taken from the following frames, so the bridge gets a corrupted script or message, and every frame after it is misread. Since #725 only that client is affected, but its pending calls are lost without a close event.

Several frames for the same client are often waiting at that point: other threads block in mg_websocket_write() on the connection lock while the first write times out, and write their frames as soon as it returns.

Fix

In _webui_send_client_ws():

  • After a write that returns -1 or fewer than packets_size bytes, the connection is marked as broken in a new internal _webui.clients_broken[] (next to clients[], under mutex_client) and closed with mg_close_connection().
  • Before writing, a connection that is marked as broken is skipped.
  • _webui_connection_save() clears the mark when a slot is (re)used, because CivetWeb reuses its struct mg_connection for later connections.

The page then sees the connection close, and the bridge reconnects (#wsStayAlive is true by default). WebUI's normal close path runs unchanged: CivetWeb calls the close handler, _webui_connection_remove() frees the slot, the window gets WEBUI_EVENT_DISCONNECTED, and waiting webui_script() calls fail via _webui_script_client_closed().

Successful writes behave as before. Nothing changes in src/civetweb/ or include/webui.h, and no exported function, type or value changes.

Thread safety

  • The check, the write and the close run while holding mg_lock_connection(client). That is the lock mg_websocket_write() already takes for every frame (civetweb.h documents it for server-initiated websocket writes), and it is recursive (PTHREAD_MUTEX_RECURSIVE / a critical section on Windows). A sender that was waiting for the connection therefore sees the broken mark and writes nothing. The lock is per connection, so as after Fix one stalled client blocking WebSocket writes to all windows #725 a stalled client blocks only writes to itself.
  • mutex_client is taken inside the connection lock only for the read and the update of the mark. No code holds mutex_client while it takes a connection lock (_webui_connection_remove() releases it before mg_close_connection()), so this lock order can't deadlock.
  • mg_close_connection() is the call WebUI already uses to close clients from other threads (webui_close_client(), _webui_connection_remove()). While the connection's thread is in its read loop it only sets must_close (civetweb.c:18441). The loop checks that flag after each read, so CivetWeb's thread leaves the loop itself, calls the close handler and closes the socket. If the loop has already ended (the client closed at the same moment), mg_close_connection() closes the socket at once, under the connection lock, and CivetWeb's own close later finds it closed. webui_close_client() behaves the same way in that case.
  • CivetWeb has to take the connection lock in close_connection() before it can reuse the connection struct. The sender holds that lock from the write to the close, so the close always applies to the connection that got the cut frame.
  • No CLOSE frame is sent: after a cut frame it would be read as payload.

One limitation: the close takes effect when CivetWeb's read wait for that connection returns, which is when the client sends anything (the bridge sends a keep-alive at least every 20 s while the page runs) or after websocket_timeout_ms (3600 s in WebUI). No frames are written to the connection in the meantime. CivetWeb has no public API for shutting down the socket of a connection that is in its read loop from another thread.

Testing

Linux x64, gcc 16.2 (make, make debug) and clang 21 (compile only): no new warnings (the only warning, system() unused result, is also on main). examples/C/minimal builds.

Reproducer (not part of this PR): a C app with multi_client that calls webui_run() with 1 MiB scripts from 2 threads, and a raw WebSocket client that does the bridge's token check, reads 3 frames, stops reading for 45 s (still sending the bridge's ping every 5 s), then reads again and checks every frame.

  • Before: the first write returns after 32 s with part of a frame sent. When the client reads again, the cut frame's payload continues with the next frame's header (82 7f 00 00 00 00 00 10 00 09 dd ... at byte 751,536 of the script) and the following frames are out of step. 3 of 3 runs.
  • After: the write returns after 32 s, the waiting second writer skips its frame, and the connection closes 3 s later at the client's next keep-alive (WEBUI_EVENT_DISCONNECTED in the app). The client gets the start of the cut frame and then the end of the connection (a reset, as its next keep-alive went to the closed socket). No further frames are written. 3 of 3 runs.
  • Without a stall (control), 15 s of reading: 1,840 frames before and 1,838 after, all intact, and the connection stays open.
  • One ASan/UBSan run of the stalled case with the fix: no findings.
  • One TSan run of the stalled case before and after. Main has one report (master_thread_run() / mg_stop()). The fix adds one: mg_close_connection() writes the plain int must_close flag while read_websocket() reads it (civetweb.c:18443 / :13526). That is how CivetWeb closes a connection from another thread, and webui_close_client() writes the same flag the same way. There are no reports in the new WebUI code.

🤖 Generated with Claude Code

`_webui_send_client_ws()` only logged the result of `mg_websocket_write()`.
When a client stops reading for longer than `request_timeout_ms` (30 s), or
the socket fails, CivetWeb returns after writing only part of a frame. The
connection stayed open and the next frames followed the partial one, so the
client read them out of step.

A failed or short write now marks the connection as broken, so nothing more
is written to it, and closes it with `mg_close_connection()`. The check, the
write and the close run under the connection's own (recursive) CivetWeb
lock, so no global lock is held during the write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AlbertShown
AlbertShown merged commit 1610871 into webui-dev:main Oct 26, 2026
@AlbertShown

Copy link
Copy Markdown
Member

Thank you @ViktorJannicke

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants