Repository navigation
Allow using multiple instances of replxx inside one program - #47
Merged
Merged
Conversation
Three pieces of mutable state were process-global: - read_unicode_character kept the partial UTF-8 sequence in function statics. It cannot be a plain local: read_verbatim switches the fd to O_NONBLOCK and a chunk boundary may fall inside a multibyte character, so the partial bytes must survive the EAGAIN return into the next blocking read_char. But it must not be shared between instances driven from different threads: two sessions typing multibyte input interleave their partial sequences and both decode garbage. - EscapeSequenceProcessing::thisKeyMetaCtrl accumulates modifiers while a CSI sequence is parsed; concurrent sessions turn Ctrl+Left into Left and vice versa. - ansi_color returns a pointer into a static buffer; concurrent highlighting emits a garbled escape sequence into another session's output. ClickHouse embeds one Replxx instance per SSH/web-terminal session, each driven by its own thread inside the server process. TSan reports the first one as a data race on `read_unicode_character(int)::utf8Count`.
The partial multibyte sequence belongs to the descriptor it was read from, not to the calling thread: one thread may drive several Replxx instances over different descriptors. Make the decoder a Terminal member and pass the Terminal through the escape-sequence dispatch chain instead of the raw input descriptor. thisKeyMetaCtrl and the ansi_color buffer stay thread_local: both are scratch state of a single call chain on one thread, never carried across calls.
Every Terminal published itself to the process-global handler pointer on raw-mode entry and cleared it on exit, so with several instances the last one won and any exit cleared it for all; the writes also raced. Only the instance that installed the handler now sets the pointer, it is cleared only if still pointing at that instance, and the pointer is atomic.
The handler stayed installed after the instance it dereferences was destroyed. Save the previous disposition on install and restore it in the destructor before the interrupt pipe is closed.
This was referenced Oct 13, 2026
Merged
`master` got a simpler version of the same fix (#44) and the esc delay support. The conflicts are resolved in favor of this branch: the UTF-8 decoder state is kept in `Terminal`, the escape sequence routines take the `Terminal`, and the `SIGWINCH` disposition is restored. The esc delay parameter from `master` is passed through, `Terminal::read_unicode_character` waits with `poll` as on `master`, and `read_verbatim` still does not wait after the first character. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
|
🕵 Merged Checked with ClickHouse built against this commit: the build passes, Ctrl-V works in |
alexey-milovidov
approved these changes
Oct 13, 2026
alexey-milovidov
added a commit
to ClickHouse/ClickHouse
that referenced
this pull request
Oct 13, 2026
The previous `replxx` commit had a regression from the esc delay support in `replxx` `master`: Meta with escape sequences (e.g. Meta+Up) was not recognized, and `04909_client_hints_history_navigation` (`meta_up_recall_no_hints`) failed in every stateless test job: https://s3.amazonaws.com/clickhouse-test-reports/praktika.html?PR=125510&sha=33773832dc78e5a5e773171f8b489ca21d9bd77a&name_0=PR&name_1=Stateless%20tests%20%28arm_binary%2C%20parallel%29 #125510 The new commit is current `replxx` `master` (which also has the UTF-8 decoder state kept per `Terminal` from ClickHouse/replxx#47) plus the fix. Related: ClickHouse/replxx#48 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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.
Changes
staticwiththread_local staticstaticinto class members (since it is a property of an object, different objects should have different state)SIGWINCH(we will disable it for web terminal)