Skip to content

Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll()) - #105559

Merged
azat merged 9 commits into
ClickHouse:masterfrom
azat:fix-replxx-crash
May 24, 2026
Merged

azat merged 9 commits into
ClickHouse:masterfrom
azat:fix-replxx-crash

Conversation

@azat

@azat azat commented May 21, 2026 •

Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll()). Note with enabled SSH/web terminal it may crash the server

ASan: https://pastila.nl/?000a2333/ec0eeeeba6505d52399343e5b03ff041#zHm494mXb7GNci6N9jePGw==GCM
Refs:

Version info

  • Merged into: 26.6.1.116
  • Backported to: 26.5.2.5, 26.4.4.13, 26.3.13.9

@clickhouse-gh

clickhouse-gh Bot commented May 21, 2026 •

Copy link
Copy Markdown
Contributor

Workflow [PR], commit [a73bb97]

Summary: ❌

job_name test_name status info comment
Stateless tests (arm_asan_ubsan, targeted) FAIL
02156_async_insert_query_log FAIL cidb
Unit tests (asan_ubsan, function_prop_fuzzer) FAIL
FunctionsStress.stress FAIL cidb, issue
AllTests FAIL cidb
Finish Workflow FAIL
python3 ./ci/jobs/scripts/workflow_hooks/check_report_messages.py FAIL

AI Review

Summary

This PR fixes high-fd terminal handling by switching select-based readiness checks to poll in both replxx::Terminal::wait_for_input (via contrib/replxx update) and LineReader::hasInputData, and adds a focused integration regression test for interactive SSH PTY behavior under large fd pressure. The previously raised issues (POLLNVAL handling and false-positive test assertion) are fixed in the current code, and I did not find any remaining correctness, safety, or evidence gaps.

Final Verdict

Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added pr-bugfix Pull request with bugfix, not backported by default submodule changed At least one submodule changed in this PR. labels May 21, 2026
@azat
azat force-pushed the fix-replxx-crash branch from 47bf353 to 98ce7de Compare May 21, 2026 16:14
Comment thread contrib/replxx Outdated
@alexey-milovidov alexey-milovidov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label May 21, 2026
Comment thread tests/integration/test_ssh/test.py Outdated
azat added 5 commits May 22, 2026 12:51
When _hold_server_fds returns, the kernel has completed all 1500 TCP
handshakes, but Poco's TCPServer has only one accept thread that does
accept()+dispatch() per iteration. The remaining connections sit in
the kernel accept queue with no userspace fd allocated, so an
immediate /proc/<pid>/fd count sees only those accepted so far (~700
in the failing run). Verified with pdb: pausing after the failure and
re-checking the count shows it grew to the expected ~1500.

Poll the fd count until it crosses the threshold instead of reading
it once, so the test does not race Poco's accept loop.
@azat
azat requested a review from alexey-milovidov May 23, 2026 11:13
@clickhouse-gh

clickhouse-gh Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.20% +0.10%
Functions 91.40% 91.40% +0.00%
Branches 76.50% 76.50% +0.00%

Changed lines: 100.00% (2/2) · Uncovered code

Full report · Diff report

@azat
azat enabled auto-merge May 24, 2026 10:24
@azat

azat commented May 24, 2026 •

Copy link
Copy Markdown
Member Author

Since this has been already deployed to a few instances, I will merge it (CI failures are not related)

robot-ch-test-poll4 added a commit that referenced this pull request May 24, 2026
Cherry pick #105559 to 26.5: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
robot-clickhouse added a commit that referenced this pull request May 24, 2026
…ZE limitation of select() (replace with poll())
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 24, 2026
azat added a commit that referenced this pull request May 24, 2026
Backport #105559 to 26.5: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
robot-clickhouse-ci-1 added a commit that referenced this pull request May 24, 2026
Cherry pick #105559 to 25.8: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
robot-clickhouse added a commit that referenced this pull request May 24, 2026
…ZE limitation of select() (replace with poll())
robot-clickhouse-ci-1 added a commit that referenced this pull request May 24, 2026
Cherry pick #105559 to 26.3: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
robot-clickhouse added a commit that referenced this pull request May 24, 2026
…ZE limitation of select() (replace with poll())
robot-clickhouse-ci-1 added a commit that referenced this pull request May 24, 2026
Cherry pick #105559 to 26.4: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
robot-clickhouse added a commit that referenced this pull request May 24, 2026
…ZE limitation of select() (replace with poll())
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label May 24, 2026
fm4v added a commit that referenced this pull request May 25, 2026
Backport #105559 to 26.3: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
fm4v added a commit that referenced this pull request May 25, 2026
Backport #105559 to 26.4: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
DavidHe-2008 pushed a commit to DavidHe-2008/ClickHouse that referenced this pull request Jun 1, 2026
Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
zeekay pushed a commit to hanzoai/datastore that referenced this pull request Sep 11, 2026
…g FD_SETSIZE limitation of select() (replace with poll())
zeekay pushed a commit to hanzoai/datastore that referenced this pull request Sep 11, 2026
…5559

Backport ClickHouse#105559 to 26.4: Fix crash in replxx due to hitting FD_SETSIZE limitation of select() (replace with poll())
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo submodule changed At least one submodule changed in this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants