Fix flaky endpoint tests and misleading core backtraces

[chenBright]

What problem does this PR solve?

Issue Number: N/A

Problem Summary:

EndPointTest.tcp_connect and EndPointTest.interrupt depended on public hosts
and network timing, causing intermittent CI failures. After an assertion
failure, the test runner could symbolize a core left by a passing death-test
child, making the failure appear to be a crash.

Observed in this CI run.

What is changed and the side effects?

Changed:

• Replace public hosts with loopback listeners using dynamically assigned ports.
• Verify successful connections and preservation of blocking and nonblocking
  socket modes, with and without connection deadlines.
• Use empty pipes to test wait timeouts and successful wait completion while
  delivering signals, restoring the previous SIGURG handler afterward.
• Clear stale cores between test binaries and symbolize only a core matching
  the failed test process's PID. Accept core.%e.%p (the CI format) and core.%p.
• Preserve the test process's standard input and original exit status, and
  quote the program path passed to gdb.

Side effects:

• Performance effects: No production library changes. The affected tests no
  longer depend on public network access or TCP accept-queue capacity.
• Breaking backward compatibility: No library API or behavior changes.
  Core symbolization requires a filename containing the failed process's PID.

---

Check List:

• Compiles with CMake and unit tests enabled on macOS.
• All 21 EndPoint tests pass for 100 iterations in shuffled order.
• All 51 shell regression checks pass across sh, bash, and dash, including
  filenames with spaces, parent/child core selection, exit status, and stdin.
• Follow the Contributor Covenant Code of Conduct.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

6 open findings

• pthread_fd_wait is forward-declared with unsigned events, but on macOS EVFILT_READ is a… · New
• WaitForPipe has no return path when neither OS_LINUX nor OS_MACOSX is defined, which will… · New
• pthread_fd_wait is forward-declared with unsigned events, but on macOS EVFILT_READ is a… · New
• This thread function uses a gtest assertion (EXPECT_EQ) from a non-test thread; gtest reporting… · New
• Restoring the previous SIGURG handler uses EXPECT_EQ, so a failure here won’t fail-fast and may… · New
• This relies on rc being set and numeric in the caller; if rc is empty/non-numeric, [ will… · New

What changed in this PR

This PR reduces endpoint test flakiness by removing dependencies on public hosts/network latency, and improves CI failure diagnostics by avoiding misleading backtraces from unrelated core files.

Changes:

• Rework EndPointTest.tcp_connect/interrupt to use loopback listeners, deterministic timeouts, and EINTR retry validation.
• Add Linux-only coverage for connect timeouts with a full accept queue.
• Update test runner backtrace printing to trigger only on signal-based exits.

File	Description
test/​run_tests.sh	Only print core backtraces for signal exits to avoid misleading output from death-test cores.
test/​endpoint_unittest.cpp	Replace public-host networking with local listeners; add deterministic timeout + EINTR behavior tests.

🧠 Review effort: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open findings

• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be… · New
• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be… · New
• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be… · New
• Using CHECK_EQ in a unit test cleanup path can abort the entire test process (and mask the… · New
• The current heuristic treats any numeric exit status > 128 as a signal exit, but processes can also… · New

6 resolved since last review

• pthread_fd_wait is forward-declared with unsigned events, but on macOS EVFILT_READ is a…
• WaitForPipe has no return path when neither OS_LINUX nor OS_MACOSX is defined, which will…
• pthread_fd_wait is forward-declared with unsigned events, but on macOS EVFILT_READ is a…
• This relies on rc being set and numeric in the caller; if rc is empty/non-numeric, [ will…
• Restoring the previous SIGURG handler uses EXPECT_EQ, so a failure here won’t fail-fast and may…
• This thread function uses a gtest assertion (EXPECT_EQ) from a non-test thread; gtest reporting…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings

• "$2" here is an exit status (typically 128 + signum for signal termination), but `kill -l… · New
• This introduces a local forward declaration of butil::pthread_fd_wait. If the real function… · New
• On macOS, EVFILT_READ is a negative constant, so casting it to unsigned changes its value… · New

5 resolved since last review

• The current heuristic treats any numeric exit status > 128 as a signal exit, but processes can also…
• Using CHECK_EQ in a unit test cleanup path can abort the entire test process (and mask the…
• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be…
• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be…
• Several new/updated checks rely on ~50ms timeouts. On heavily contended CI machines, 50ms can be…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open findings

• These tests rely on forward declarations of non-public functions (pthread_fd_wait,… · New
• On loopback, successful connects should complete quickly. Using a 5s timeout increases worst-case… · New
• When tcp_listen fails, this assertion provides no errno context, making CI failures harder to… · New
• This global has external linkage as written. Since it’s only used within this translation unit,… · New
• expr spawns an external process and is unnecessary here. Prefer shell arithmetic expansion (e.g.,… · New

3 resolved since last review

• "$2" here is an exit status (typically 128 + signum for signal termination), but `kill -l…
• On macOS, EVFILT_READ is a negative constant, so casting it to unsigned changes its value…
• This introduces a local forward declaration of butil::pthread_fd_wait. If the real function…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings

• pthread_fd_wait_impl is a good refactor, but the new non-static macOS overload introduced below… · New
• The test adds manual forward declarations for butil::pthread_fd_wait (including a macOS-only… · New
• signal_number is introduced as a function-scoped working variable, but in shell functions… · New

5 resolved since last review

• On loopback, successful connects should complete quickly. Using a 5s timeout increases worst-case…
• These tests rely on forward declarations of non-public functions (pthread_fd_wait,…
• expr spawns an external process and is unnecessary here. Prefer shell arithmetic expansion (e.g.,…
• This global has external linkage as written. Since it’s only used within this translation unit,…
• When tcp_listen fails, this assertion provides no errno context, making CI failures harder to…

🧠 Review effort: Lite

[Copilot]

🟡 Changes recommended

The interruption test can pass without exercising signal retries, and the runner can still misidentify a normal exit as a signal failure.

2 open findings

• Test may signal before the worker enters the active wait · New
• Shell status conflates normal exit with SIGTERM termination · New

3 resolved since last review

• The test adds manual forward declarations for butil::pthread_fd_wait (including a macOS-only…
• pthread_fd_wait_impl is a good refactor, but the new non-static macOS overload introduced below…
• signal_number is introduced as a function-scoped working variable, but in shell functions…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

• The test references butil::g_pthread_fd_wait_eintr_count, but src/butil/endpoint.cpp only… · New
• The test references butil::g_pthread_fd_wait_eintr_count, but src/butil/endpoint.cpp only… · New
• On non-Linux and non-macOS builds, this function has no return path (control reaches end of… · New
• The revised core lookup assumes the core naming format core.%e.%p and will miss valid cores on… · New

2 resolved since last review

• Shell status conflates normal exit with SIGTERM termination
• Test may signal before the worker enters the active wait

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

10 open findings

• These headers are included unconditionally, which can break builds on non-POSIX platforms (e.g.,… · New
• This test’s determinism depends on kernel-specific listen(backlog=0) semantics and sysctls like… · New
• These newly introduced test-only helpers have external linkage inside namespace butil, which can… · New
• These newly introduced test-only helpers have external linkage inside namespace butil, which can… · New
• These newly introduced test-only helpers have external linkage inside namespace butil, which can… · New
• These newly introduced test-only helpers have external linkage inside namespace butil, which can… · New
• These newly introduced test-only helpers have external linkage inside namespace butil, which can… · New
• This file includes butil/memory/scope_guard.h, but uses the BRPC_SCOPE_EXIT macro. If… · New
• The PR description says 'Print core backtraces only for signal exit statuses', but the updated… · New
• Quote the program path when invoking gdb (use \"$1\") to avoid argument-splitting issues if the… · New

4 resolved since last review

• The test references butil::g_pthread_fd_wait_eintr_count, but src/butil/endpoint.cpp only…
• The test references butil::g_pthread_fd_wait_eintr_count, but src/butil/endpoint.cpp only…
• The revised core lookup assumes the core naming format core.%e.%p and will miss valid cores on…
• On non-Linux and non-macOS builds, this function has no return path (control reaches end of…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

4 open findings

• These manual forward declarations can silently drift from the real definitions (or conflict with a… · New
• On macOS, EVFILT_READ/EVFILT_WRITE are negative filter values. Casting them to unsigned and… · New
• Restricting symbolization to cores named with the failing test process PID avoids stale death-test… · New
• This comment is inaccurate: g_sigurg_received is written by the SIGURG handler (signal… · New

10 resolved since last review

• These headers are included unconditionally, which can break builds on non-POSIX platforms (e.g.,…
• The PR description says 'Print core backtraces only for signal exit statuses', but the updated…
• This file includes butil/memory/scope_guard.h, but uses the BRPC_SCOPE_EXIT macro. If…
• These newly introduced test-only helpers have external linkage inside namespace butil, which can…
• These newly introduced test-only helpers have external linkage inside namespace butil, which can…
• These newly introduced test-only helpers have external linkage inside namespace butil, which can…
• These newly introduced test-only helpers have external linkage inside namespace butil, which can…
• These newly introduced test-only helpers have external linkage inside namespace butil, which can…
• This test’s determinism depends on kernel-specific listen(backlog=0) semantics and sysctls like…
• Quote the program path when invoking gdb (use \"$1\") to avoid argument-splitting issues if the…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open findings

• This test creates multiple client connections to a listening socket without ever calling accept()… · New
• find ... -printf is a GNU find extension and is not supported by BSD/macOS find. If this script… · New

4 resolved since last review

• Restricting symbolization to cores named with the failing test process PID avoids stale death-test…
• On macOS, EVFILT_READ/EVFILT_WRITE are negative filter values. Casting them to unsigned and…
• These manual forward declarations can silently drift from the real definitions (or conflict with a…
• This comment is inaccurate: g_sigurg_received is written by the SIGURG handler (signal…

🧠 Review effort: Lite

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open findings

• Casting EVFILT_READ (typically a negative int) to unsigned is non-obvious and can be… · New
• This introduces a new timing-sensitive assertion with a relatively short (300ms) deadline. If the… · New
• rm -f core.* only clears cores in the current directory, but find . will also search… · New
• Redirecting FD 3 on the loop terminator (done 3<&0) is valid but non-obvious, especially combined… · New
• Redirecting FD 3 on the loop terminator (done 3<&0) is valid but non-obvious, especially combined… · New

2 resolved since last review

• find ... -printf is a GNU find extension and is not supported by BSD/macOS find. If this script…
• This test creates multiple client connections to a listening socket without ever calling accept()…

🧠 Review effort: Lite

[chenBright]

I believe this PR is ready for a final human review. I’ve evaluated the Copilot feedback and fixed the confirmed issues. The remaining comments concern existing API design, stylistic preferences, or assumptions that do not apply to these changes.

The changes are limited to tests and the test runner, with no production code changes. The build, endpoint tests, and shell regression checks have passed.

@wwbmmm PTAL.