WebSockets server: fix crash when enabled from the menu, 16 kB payload cap, duplicate endpoint descriptors - #115
Merged
slajerek merged 1 commit intoAug 4, 2026
Conversation
…descriptor duplicates Three defects in the WebSockets debugger server: 1. Enabling the server from Settings -> Emulation killed the process instantly, with no dialog and nothing in the log. DebuggerServerWebSocketsStart() both creates AND starts the server, and the menu handler then called Start() a second time; SYS_StartThread() answers a double start with SYS_FatalExit(). The existing guard could not catch it: both serverStarted and isRunning are set by the server thread once it is already up, so two calls in a row see them false. Added startRequested, set synchronously in Start(), and made the menu handler start the server only once. 2. A payload of 16 kB or more was dropped by closing the socket with no error and no close frame -- uWS defaults maxPayloadLength to 16 * 1024. A client sending a PRG or a disk image in one writeBlock just lost the connection with no way to tell why. Raised to 4 MB. 3. endpointDescriptors had no dedupe while the handler map is keyed by fn, so a re-registration left a stale twin behind. server/capabilities reported 78 endpoints for c64 while 77 were reachable, and every Stop()+Start() cycle grew the list further. Verified on Debian 13: a 32 kB writeBlock round-trips intact, and server/capabilities now agrees with server/endpoints (78 = 78).
Owner
|
Swoją drogą, ten 16kB limit już mam fix na priv repo, ale to jeszcze in progress... :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent defects in the WebSockets debugger server, found while driving the debugger
from scripts. They are grouped because they all live in
CDebuggerServerWebSockets.1. Enabling the server from the menu kills the process
Clicking Settings -> Emulation -> "WebSockets debugger server" made the application vanish
instantly: no dialog, no message box, nothing in the log. Starting the server from
settings.hjsoninstead was always healthy, which pointed at the enable-at-runtime path.CViewC64::DebuggerServerWebSocketsStart()(CViewC64.cpp:4480-4497) both creates andstarts the server. The menu handler then called
Start()again on the object it had justcreated, and a second
SYS_StartThread()on a runningCSlrThreadends inSYS_FatalExit("thread %d is already running")-- an immediate exit. WithGLOBAL_DEBUG_OFFthe
LOGErrorabove it compiles to nothing, so the log stays empty.The existing
if (serverStarted)guard cannot catch this:serverStartedis set insideThreadRun()andCSlrThread::isRunningis set by the newly spawned thread, so two calls ina row both observe them as false. This adds
startRequested, set synchronously insideStart(), and makes the menu handler start the server only once.2. Payloads of 16 kB or more silently kill the connection
uWS::WebSocketBehaviordefaultsmaxPayloadLengthto16 * 1024(App.h:251) and theserver never overrides it. Above that, uWS force-closes the socket without an error and
without a close frame -- the client just sees the connection drop. Measured: the largest
payload that gets through is 16300 bytes.
In practice that means a PRG or a disk image cannot be sent in one
writeBlock, and there isno feedback explaining why. Raised to 4 MB, which covers 64 kB of RAM, D64, D81 and CRT with
room to spare. The field must sit before
.idleTimeoutto satisfy C++20 designatedinitializer ordering.
3.
endpointDescriptorshad no dedupeAddEndpointFunction(desc, handler)writes the handler into a map keyed byfn(so a secondregistration replaces it) but unconditionally
push_backs the descriptor. The two containersthen disagree. Today this is visible as
server/capabilitiesreportingendpointCount78 forc64 while only 77 endpoints are reachable --
c64/drive1541/ram/clearis registered twice(
CDebuggerServerApiVice.cpp:454and:735).It also grows without bound: registration runs inside
ThreadRun(), so everyStop()+Start()cycle appends a fresh copy of every descriptor. The fix replaces an existing entrywith the same
fnin place, keeping first-registration order and last-wins metadata,consistent with how the handler map already behaves.
Verification
Debian 13, same binary before/after:
ram/writeBlockcapabilitiesvsendpoints(c64)The crash was verified by hand after the fix: toggling the menu item off and on repeatedly,
plus restarting the application with the setting both
falseandtrueand toggling again --no crash in any of those. Before the fix, enabling it from the menu killed the process every
time. Note when reproducing: the run has to start with
RunDebuggerServerWebSockets: false,otherwise the first click disables the server and never reaches the double-start path.