Skip to content

Pin wire-sys-vm 1.2.0#2: read-only shared-code SIGSEGV fix - #703

Merged
heifner merged 2 commits into
masterfrom
pin/wire-sys-vm-1.2.0-2
Oct 24, 2026
Merged

heifner merged 2 commits into
masterfrom
pin/wire-sys-vm-1.2.0-2

Conversation

@heifner

@heifner heifner commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Consumes Wire-Network/wire-vcpkg-registry#23 (merged as 81b3b452 on top of #22): registry baseline/reference → 81b3b452, which carries both softfloat 3.0.0#3 (#701) and wire-sys-vm 1.2.0#2; wire-sys-vm override → 1.2.0#2 (Wire-Network/eos-vm@f760622, eos-vm#14 with #15).

Why

read-only-trx-irreversible-test timeoutTest aborted nodeop with SIGSEGV in CI run 39807473127. Read-only threads share one compiled sys-vm JIT module; a timed-out run revoked its code pages while a peer's timed_run exit cleared the process-wide timeout flag, so the faulting thread was handed to the previous SIGSEGV handler. Any API node with more than one read-only thread can hit it. The pinned eos-vm classifies faults in generated code by location, serializes timed_run registration and exit transitions under a mutex (timeout publication stays lock-free in signal context), and backports the upstream signal-mask and <cstdint> fixes.

Verification

A fresh worktree build configured against registry 81b3b452 with both overrides: vcpkg installs [email protected]#3 (exporting THREAD_LOCAL=__thread) and [email protected]#2 (port tree 4aca5b06), and the installed sysio/vm headers carry the fix. Read-only test matrix on the final headers (CI gcc-14 chroot, unit/plugin read-only and checktime suites under all three runtimes, the read-only/interrupt cluster tests repeated until-fail:3, both perf tests): all green on the final headers — unit and plugin suites rc=0 under sys-vm, sys-vm-jit and sys-vm-oc, 11/11 ctest entries in pass 1, the 8 read-only/interrupt cluster tests 3/3 each, both perf tests passed, no crash signatures in any log.

Regression test (second commit)

read-only-trx-irreversible-test could pass after the API node died: pushTransaction turns every clio failure, a refused connection included, into a (False, message) result; the "should fail" cases accepted any false result; worker threads reported through one shared errorInThread flag that every worker reset and that two call sites never checked, so a failing thread only printed a traceback; nothing checked that the node was still running. tests/read_only_trx_test.py now runs workers on a WorkerThread that keeps its exception (joinAll fails the test on any of them), validates expected failures by exception code (tx_cpu_usage_exceeded / deadline_exception for the forever and infinite transactions and for fast or slow ones that lose to the read window, wasm_execution_error for the segv action), and asserts apiNode.verifyAlive() after every concurrent phase and after the segv cases.

Checked against the pinned build: the irreversible variant passes (-p 2 -n 3 --read-only-threads 2 --api-node-read-mode irreversible); the 16-thread parallel variant passes (--read-only-threads 16 --num-test-runs 2); a run in which the API node is sent SIGSEGV during timeoutTest fails within two seconds of the kill: worker thread failures: … clio failed without the expected exception, is nodeop still up? Failed http request to nodeop ….

Overlap

#701 (softfloat 3.0.0#3) edits the same two files; whichever lands second re-pins to a registry commit that carries both bumps.

@huangminghuang

Copy link
Copy Markdown
Contributor

Medium — Make the regression test fail if nodeop exits

This PR pins the dependency to fix a SIGSEGV reproduced by read-only-trx-irreversible-test, but that test can still report success after the process dies:

  • transactions.py:253-265 converts every CLI failure, including connection refusal after node death, into a normal (False, message) result.
  • read_only_trx_test.py:428-442 accepts any false result as the expected timeout/error.
  • Worker threads are only joined at lines 471-484, so their assertion failures do not propagate, and the main path later marks the test successful at line 507.

This occurred in the GCC CI job: the test artifact records repeated connection refusals followed by a passing result, while the accompanying core artifact contains a nodeop core generated during the test. An ineffective dependency pin could therefore still merge green.

Please propagate worker failures, validate the specific expected timeout/VM error instead of any false result, and assert apiNode.verifyAlive() after the concurrent workload.

heifner added a commit that referenced this pull request Oct 23, 2026
Review of #703 (huangminghuang): the test could pass after the API node died. pushTransaction turns every clio failure, a refused connection included, into a (False, message) result; the "should fail" cases accepted any false result; worker threads set a shared errorInThread flag that every worker reset and that two call sites never checked, so a failing thread only printed a traceback; and nothing checked that the node was still running. CI run 39807473127 shows the refusals and a passing result next to a nodeop core.

Workers now run on a WorkerThread that keeps its exception, and joinAll fails the test on any of them. Expected failures are validated by exception code: tx_cpu_usage_exceeded or deadline_exception for the forever and infinite transactions and for a fast or slow transaction that loses to the read window, wasm_execution_error for the segv action; a transport error no longer passes as an expected failure. After every concurrent phase, and after the segv cases, the test asserts apiNode.verifyAlive().

Verified against the pinned build: the irreversible and the 16-thread parallel variants pass, and a run in which the API node is sent SIGSEGV during timeoutTest fails within two seconds, naming the refused connections.

Change-Id: I35c7d0bdcfeeba57573b83ff7a3e8e3461d5bc31
@heifner

heifner commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in f7abf65 (tests/read_only_trx_test.py, second commit): worker threads run on a WorkerThread that keeps its exception and joinAll fails the test on any of them; expected failures are validated by exception code (tx_cpu_usage_exceeded / deadline_exception for the forever and infinite transactions and for fast or slow ones that lose to the read window, wasm_execution_error for the segv action), so a refused connection no longer passes as an expected failure; apiNode.verifyAlive() is asserted after every concurrent phase and after the segv cases.

Checked against the pinned build: the irreversible and the 16-thread parallel variants pass; a run in which the API node is sent SIGSEGV during timeoutTest fails within two seconds with worker thread failures: … clio failed without the expected exception, is nodeop still up? Failed http request to nodeop …. The read-only matrix on the final headers is in the PR body.

huangminghuang
huangminghuang previously approved these changes Oct 23, 2026

@huangminghuang huangminghuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the dependency pin and follow-up regression-test hardening. The worker failures now propagate, expected transaction errors are distinguished from transport failures, and API-node liveness is checked. No actionable findings remain.

Moves the registry baseline/reference to wire-vcpkg-registry@1c947d6 and the wire-sys-vm override to 1.2.0#2 (Wire-Network/eos-vm@f760622: eos-vm#14 with #15). Faults on revoked JIT code pages are classified by location instead of the process-wide timeout flag, timed_run registration and exit transitions are serialized, and the upstream signal fixes are backported, so a read-only transaction timeout no longer aborts nodeop (CI run 39807473127, read-only-trx-irreversible-test timeoutTest).

Change-Id: I73301fab0a71274dc9a2b17e98d6465e6a352a0a
Review of #703 (huangminghuang): the test could pass after the API node died. pushTransaction turns every clio failure, a refused connection included, into a (False, message) result; the "should fail" cases accepted any false result; worker threads set a shared errorInThread flag that every worker reset and that two call sites never checked, so a failing thread only printed a traceback; and nothing checked that the node was still running. CI run 39807473127 shows the refusals and a passing result next to a nodeop core.

Workers now run on a WorkerThread that keeps its exception, and joinAll fails the test on any of them. Expected failures are validated by exception code: tx_cpu_usage_exceeded or deadline_exception for the forever and infinite transactions and for a fast or slow transaction that loses to the read window, wasm_execution_error for the segv action; a transport error no longer passes as an expected failure. After every concurrent phase, and after the segv cases, the test asserts apiNode.verifyAlive().

Verified against the pinned build: the irreversible and the 16-thread parallel variants pass, and a run in which the API node is sent SIGSEGV during timeoutTest fails within two seconds, naming the refused connections.

Change-Id: I35c7d0bdcfeeba57573b83ff7a3e8e3461d5bc31

@huangminghuang huangminghuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head cca582d. No actionable findings.

@heifner
heifner merged commit 1a3f857 into master Oct 24, 2026
13 checks passed
@heifner
heifner deleted the pin/wire-sys-vm-1.2.0-2 branch October 24, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants