web: non-blocking web_server, browser console mirroring, Tcl 9 fix#10392
web: non-blocking web_server, browser console mirroring, Tcl 9 fix#10392maliberty wants to merge 1 commit into
Conversation
Make `web_server` a transition point in scripts and a blocking suspend-the-prompt call at the interactive Tcl prompt, with the rest of the script running through the now-active server and browser-driven commands serializing against the main thread. * `web_server` Tcl proc: walks `info frame` for `tclreadline::Loop` to detect interactive vs. script context. Scripts return immediately so subsequent lines run as if typed in the browser console; interactive use calls `web_server_wait_cmd` which parks the main thread in `Tcl_DoOneEvent` so the launching terminal prompt is suppressed while the server is up. The event mask excludes `TCL_FILE_EVENTS` so tclreadline's stdin handler does not keep reading the terminal. * Tcl 9 cross-thread fix: `TclEvaluator::eval` marshals browser worker `tcl_eval` requests to the main thread via `Tcl_ThreadQueueEvent` + condition variable instead of calling `Tcl_Eval` directly on the worker. Tcl 9 isolates per-thread interpreter state, so a direct worker `Tcl_Eval` fails to see globals set on the main thread (e.g. `set x 1` at the prompt followed by `puts $x` in the browser returns an unset-variable error). Evaluation uses `Tcl_EvalEx(..., TCL_EVAL_GLOBAL)` so commands run at the top-level namespace regardless of the Tcl call frame. * Browser console mirroring: a level-1 `Tcl_CreateObjTrace` broadcasts every main-thread top-level command as a `console_input` push frame, so script lines and terminal-typed commands appear in the browser Tcl console as `>>> cmd`. The trace filters by `Tcl_GetCurrentThread() == mainThreadId()` to avoid double-echoing browser-typed commands (the JS widget echoes them locally). Two `Tcl_StackChannel` transforms on stdout / stderr forward `puts`/error output through to the terminal and broadcast it to the browser as a `log` frame. * Browser-typed `exit` / `web_server -stop` wake the main thread via a no-op event queued with `Tcl_ThreadQueueEvent` + `Tcl_ThreadAlert`. The wait loop checks `exitRequested()` / `stopRequested()` and the calling site (`Main.cc` or `web_server_wait_cmd`) calls `stop()` and, on exit-request, `std::exit(0)` on the main thread. * Channel mirror passes the actual `written` count (not requested) to the broadcast so a partial write doesn't claim bytes that never reached the terminal. * `WebServer::stop()` integrates the new teardown (unstack channels, remove trace, restore `exit`, reset state) and remains callable from any thread via `stopAndJoinIoThreads`'s self-join handling. * main.js renders `console_input` push frames as `>>> cmd` lines. Tests adapted: `TclEvaluator` constructor now takes a shared `std::mutex&` and `Tcl_ThreadId`; tests pass `nullptr` for the thread id so the direct fast-path runs. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07954badef
ℹ️ 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".
| if { $in_readline } { | ||
| web::web_server_wait_cmd | ||
| } |
There was a problem hiding this comment.
Returning immediately in script mode means the main Tcl thread can keep running design-mutating commands while web worker threads continue servicing requests, but the worker-side serialization (tcl_eval_->mutex in handlers such as SelectHandler::handleSelect) does not protect against concurrent main-thread STA/DB access. This reintroduces unsynchronized cross-thread access to components explicitly marked non-thread-safe and can cause races or crashes when a script runs after web_server and a browser is connected.
Useful? React with 👍 / 👎.
| while (!exitRequested() && !stopRequested() && isRunning()) { | ||
| Tcl_DoOneEvent(kEventMask); |
There was a problem hiding this comment.
This loop ignores the return value of Tcl_DoOneEvent; when there are no eligible timer/idle/window handlers, Tcl can return 0 immediately instead of blocking, so the loop busy-spins until stopRequested/exitRequested flips. In headless web sessions this can peg a CPU core while waiting for user actions.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the web server's integration with the Tcl event loop to support Tcl 9's thread isolation and improve the interactive experience. Key changes include implementing cross-thread marshalling for Tcl commands via Tcl_ThreadQueueEvent, mirroring Tcl's stdout and stderr channels to the browser console using Tcl_StackChannel, and broadcasting top-level main-thread commands to connected clients. The WebServer API was updated to replace the blocking waitForStop with a Tcl-native event loop in runEventLoopUntilStop. Feedback identified a signature mismatch in the Tcl channel output procedure where a parameter should be char* instead of const char* to comply with the Tcl API.
| static int webChannelOutputProc(ClientData instanceData, | ||
| const char* buf, | ||
| int toWrite, | ||
| int* errorCodePtr) | ||
| { | ||
| auto* m = static_cast<WebChannelMirror*>(instanceData); | ||
|
|
||
| // Forward to the next channel in the stack so the terminal still | ||
| // sees the output. | ||
| const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent); | ||
| Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type); | ||
| ClientData parent_data = Tcl_GetChannelInstanceData(m->parent); | ||
| const int written = parent_output( | ||
| parent_data, const_cast<char*>(buf), toWrite, errorCodePtr); |
There was a problem hiding this comment.
The signature of webChannelOutputProc does not match the Tcl_DriverOutputProc type expected by Tcl_ChannelType.outputProc. The buf parameter should be char*, not const char*.
This mismatch can cause compilation errors and relies on unsafe type conversions. Correcting the signature to char* buf aligns with the Tcl API, improves type safety, and removes the need for const_cast when calling the parent channel's output procedure.
| static int webChannelOutputProc(ClientData instanceData, | |
| const char* buf, | |
| int toWrite, | |
| int* errorCodePtr) | |
| { | |
| auto* m = static_cast<WebChannelMirror*>(instanceData); | |
| // Forward to the next channel in the stack so the terminal still | |
| // sees the output. | |
| const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent); | |
| Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type); | |
| ClientData parent_data = Tcl_GetChannelInstanceData(m->parent); | |
| const int written = parent_output( | |
| parent_data, const_cast<char*>(buf), toWrite, errorCodePtr); | |
| static int webChannelOutputProc(ClientData instanceData, | |
| char* buf, | |
| int toWrite, | |
| int* errorCodePtr) | |
| { | |
| auto* m = static_cast<WebChannelMirror*>(instanceData); | |
| // Forward to the next channel in the stack so the terminal still | |
| // sees the output. | |
| const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent); | |
| Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type); | |
| ClientData parent_data = Tcl_GetChannelInstanceData(m->parent); | |
| const int written = parent_output( | |
| parent_data, buf, toWrite, errorCodePtr); |
References
- API usability and compatibility with external driver signatures (like Tcl_DriverOutputProc) can be prioritized over strict const-correctness to avoid unsafe type conversions and ensure correct linkage.
1 participant
Summary
Make
web_servera transition point in scripts and a blocking suspend-the-prompt call at the interactive Tcl prompt, with the rest of the script running through the now-active server and browser-driven commands serializing against the main thread.web_serverTcl proc: walksinfo framefortclreadline::Loopto detect interactive vs. script context. Scripts return immediately so subsequent lines run as if typed in the browser console; interactive use callsweb_server_wait_cmdwhich parks the main thread inTcl_DoOneEventso the launching terminal prompt is suppressed while the server is up. The event mask excludesTCL_FILE_EVENTSso tclreadline's stdin handler does not keep reading the terminal.Tcl 9 cross-thread fix:
TclEvaluator::evalmarshals browser workertcl_evalrequests to the main thread viaTcl_ThreadQueueEvent+ condition variable instead of callingTcl_Evaldirectly on the worker. Tcl 9 isolates per-thread interpreter state, so a direct workerTcl_Evalfails to see globals set on the main thread (e.g.set x 1at the prompt followed byputs $xin the browser returns an unset-variable error). Evaluation usesTcl_EvalEx(..., TCL_EVAL_GLOBAL)so commands run at the top-level namespace regardless of the Tcl call frame.Browser console mirroring: a level-1
Tcl_CreateObjTracebroadcasts every main-thread top-level command as aconsole_inputpush frame, so script lines and terminal-typed commands appear in the browser Tcl console as>>> cmd. The trace filters byTcl_GetCurrentThread() == mainThreadId()to avoid double-echoing browser-typed commands (the JS widget echoes them locally). TwoTcl_StackChanneltransforms on stdout / stderr forwardputs/error output through to the terminal and broadcast it to the browser as alogframe.Browser-typed
exit/web_server -stopwake the main thread via a no-op event queued withTcl_ThreadQueueEvent+Tcl_ThreadAlert. The wait loop checksexitRequested()/stopRequested()and the calling site (Main.ccorweb_server_wait_cmd) callsstop()and, on exit-request,std::exit(0)on the main thread.Channel mirror passes the actual
writtencount (not requested) to the broadcast so a partial write doesn't claim bytes that never reached the terminal.WebServer::stop()integrates the new teardown (unstack channels, remove trace, restoreexit, reset state) and remains callable from any thread viastopAndJoinIoThreads's self-join handling.main.js renders
console_inputpush frames as>>> cmdlines.Tests adapted:
TclEvaluatorconstructor now takes a sharedstd::mutex&andTcl_ThreadId; tests passnullptrfor the thread id so the direct fast-path runs.Type of Change
Verification
./etc/Build.sh).