fix(runtime): stop the cooperate checkpoint clobbering the last-error slot#2669
Open
slepp wants to merge 3 commits into
Open
fix(runtime): stop the cooperate checkpoint clobbering the last-error slot#2669slepp wants to merge 3 commits into
slepp wants to merge 3 commits into
Conversation
hew_actor_cooperate is a fail-open reduction-budget probe: with no installed execution context it returns 0 and keeps running. It used require_current_context(), which sets the "execution context not installed" sentinel into the thread-local last-error slot as a side effect. In straight-line main() the compiler-injected cooperate call at last_process_error's function entry fired after process launch had set the real failure text and before the stdlib read it back, so try_run_argv callers saw the sentinel instead of the genuine OS launch error. Switch both the native and WASM cooperate checkpoints to the silent current_context() probe — identical null-check semantics, no error write — and document that the no-context path leaves the last-error slot untouched. require_current_context() and its other callers are unchanged; they legitimately want the diagnostic.
… callers The existing missing-command test only matches the LaunchFailed variant and discards the message, so it stayed green while the cooperate checkpoint replaced the real OS launch error with the execution-context sentinel. Assert the message content: it must carry the runtime's 'failed to execute' text and must not carry the sentinel. The fixture command is a nonexistent binary, so the launch fails identically on every platform with no Unix-only tools.
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
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.
Why
The actor scheduler's cooperate checkpoint used
require_current_context(), whose null-context path has a side effect: it writes theEXECUTION_CONTEXT_NOT_INSTALLEDsentinel intoLAST_ERROR. That write clobbered genuine diagnostics — for example a real process launch failure — before stdlib callers could read them, sotry_run_argvcallers saw the context sentinel instead of the actual OS error text.What
hew_actor_cooperateinscheduler.rsandscheduler_wasm.rsnow uses the non-clobberingcurrent_context()probe. The null check is semantically identical; only the spuriousLAST_ERRORwrite is gone. Cooperate's contract is its return code — it is deliberately not a typed-error emitter, and the checkpoint never fabricates an error into the slot.task_scope.rs's test is rewritten to assert the intended behaviour (a clear slot stays clear across a no-context cooperate, and a real diagnostic set before the checkpoint survives it); the execution-context catalogue case now documents cooperate as leaving the slot untouched.Test
test_try_run_argv_missing_command_preserves_real_errorintests/hew/process_test.hew: asserts the surviving error contains the real "failed to execute" text and not the sentinel. Verified red against the pre-fix runtime, green with the fix.cooperate_without_execution_context_preserves_last_error(task_scope.rs): covers both negative-space conditions — no fabricated error on a clear slot, and a pre-set diagnostic surviving the checkpoint..hewregression was run three times to rule out order sensitivity.Quality Checklist
.ok()?orunwrap_or_default()in codegen without// JUSTIFIEDcomment// WASM-TODO(#NNN):marker (with issue ref, e.g.#1451), and newhew_*exports are classified inscripts/jit-symbol-classification.tomlOut of scope
printf/killfixture portability cleanup in the process tests.require_current_context()diagnostic callers are correct as-is (their readers want the sentinel) and are unchanged.Fixes #2506