fix(std/net): route delayed network errors to actor-scoped error slots#2667
Merged
Conversation
Re-point set_/clear_/get_tls_last_error to
hew_runtime::parse_error_slot::{set_error,clear_error,get_error}(ErrorSlotKind::Tls, ...)
and delete the predecessor thread_local! LAST_TLS_ERROR. The public
hew_tls_last_error() C ABI is unchanged; existing set/clear call sites
and the tls.hew status-code pin test are unaffected.
Adds a regression test proving a TLS error recorded for an actor on
one OS thread is readable for the same actor on a different OS
thread, the scenario a plain thread_local! cannot support once an
actor migrates across scheduler workers.
Refs #2659.
Re-point set_/clear_/get_smtp_last_error to
hew_runtime::parse_error_slot::{set_error,clear_error,get_error}(ErrorSlotKind::Smtp, ...)
and delete the predecessor thread_local! LAST_SMTP_ERROR. The public
hew_smtp_last_error() C ABI is unchanged; existing send-path callers
(smtp_error_result, hew_smtp_connect(_tls), smtp_send_impl) route
through the same helpers unmodified.
Adds a regression test proving an SMTP error recorded for an actor on
one OS thread is readable for the same actor on a different OS
thread.
Refs #2659.
…_error_slot
Re-point set_/clear_/get_constructor_last_error to
hew_runtime::parse_error_slot::{set_error,clear_error,get_error}(ErrorSlotKind::Quic, ...)
and delete the predecessor thread_local! LAST_CONSTRUCTOR_ERROR. The
public hew_quic_last_error() C ABI is unchanged; the constructor call
sites (set_constructor_error_and_return, hew_quic_new_client(_with_ca),
hew_quic_new_server(_with_tls)) route through the same helpers
unmodified.
Scoped to the constructor slot only. The per-endpoint/per-connection
state.last_error path (connection_last_error, hew_quic_endpoint_last_error)
is already actor-safe (stored in the handle struct, not thread-local)
and is left untouched.
Adds a regression test proving a QUIC constructor error recorded for
an actor on one OS thread is readable for the same actor on a
different OS thread.
Refs #2659.
…ter checks The prior actor-drift regression tests for TLS/SMTP/QUIC wrote and read the shared parse_error_slot map directly via the __set_error_for_actor/ __get_error_for_actor test-only helpers, bypassing each protocol's own setter/producer and public getter. That let the tests pass even against the pre-migration thread_local! implementation, since they never exercised the code path the migration actually changed. Rewrite each test to install a real actor as the current dispatch context (via hew_actor_spawn + hew_runtime::set_current_context) on one OS thread, drive the actual failure path (hew_tls_connect, hew_smtp_send, hew_quic_new_server), then install the same actor as the dispatch context on a different OS thread and read the error back through the real public getter (hew_tls_last_error, hew_smtp_last_error, hew_quic_last_error). This is genuinely red against a thread_local! implementation and green only when the getter is actor-scoped. Verified empirically for each protocol: temporarily reverted the migrated set/clear/get helpers to a thread_local! implementation, confirmed the rewritten test fails with an empty string on the second thread, then restored the migration and confirmed the test passes.
…elper Each of the tls/smtp/quic actor-drift regression tests (#2659) had grown its own module-local scheduler-guard mutex and its own install-call-restore context helper. Three independent locks do not serialize against each other: hew_sched_shutdown/hew_runtime_cleanup reclaim every tracked actor process-wide, so a TLS regression test's teardown could race a concurrently running SMTP or QUIC regression test's still-live actor. The install-call-restore context helper was also not unwind-safe — a panic inside the installed scope would skip the restore call and leave a stale actor pointer as the current dispatch context. Extract both into one shared, cfg(test)-only module (net_error_slot_test_support) reused by all three protocols: - NetErrorSlotRuntimeGuard: a single process-wide lock/guard, so every drift regression test across tls/smtp/quic mutually excludes every other one on the one process-global scheduler slot, not just its own siblings within the same file. - ActorContextGuard / with_actor_context: an RAII install/restore guard. Restoration now happens in Drop, which Rust runs during unwinding, so a panicking producer call still restores the previous thread-local context before the unwind continues. - spawn_error_slot_test_actor: the no-op-dispatch test-actor spawn, identical across all three protocols, kept in one place. Verified the extraction preserves behavior: re-ran the red/green proof for tls (temporarily reverted set_/clear_/get_tls_last_error to a thread_local!, confirmed the test fails, restored, confirmed it passes) and confirmed the panic-inside-with_actor_context path restores the prior context via a temporary, non-committed probe test before removing it. All three regression tests also pass under plain multi-threaded cargo test (not just nextest's per-test process isolation), directly exercising the new shared lock under real concurrency. No production code changed — parse_error_slot migration behavior is unchanged; this is test-harness-only.
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
1 participant
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.
Summary
TLS, SMTP, and QUIC delayed/constructor errors now route to the actor-scoped
parse_error_slotinstead of process-global last-error state. Previously, concurrent actors performing network operations could clobber each other's error reporting: actor B's failure would overwrite the error actor A was about to read viahew_tls_last_error/hew_smtp_last_error/hew_quic_last_error. Each actor now sees only its own delayed network errors.Regression tests drive real failing producers (
hew_tls_connect,hew_smtp_send,hew_quic_new_server) under actor A and read the public last-error getters under the same actor from another thread, with shared runtime-guard and unwind-safe actor-context test scaffolding to keep the concurrent tests isolated from each other's teardown.Verification
cargo nextest run --workspace: 11534/11534 passedcargo clippy -D warnings: cleancargo fmt --check: cleanmake test-stdlib: 72/72 passedcargo test -p hew-std regression_2659 -- --test-threads=3: all three regression tests pass concurrently (run 3x)cargo build --target wasm32-wasip1: builds cleanhew-stdfiles (+376/−24)Out of scope
state.last_errorbehaviour is untouched — this change covers the delayed/constructor error path only.#[cfg(all(test, not(target_family = "wasm")))].Fixes #2659