Restore a native call's copied first argument on unwind by yuvalrakavy · Pull Request #1189 · rhaiscript/rhai

yuvalrakavy · 2026-10-16T17:51:20Z

Fixes the abort reported in #1188. The sync panic behind it is split out into #1190, as requested.

exec_native_fn_call swaps a copy of the first argument in through ArgBackup when a non-method native function is called with a variable first argument, and restored it only on the normal-return path. If the native function panics, the unwind drops the ArgBackup unrestored, its Drop assertion panics during the unwind, and the process aborts, so a host's catch_unwind cannot contain it.

let mut engine = Engine::new();
engine.register_fn("buggy", |_x: INT| -> INT { panic!("buggy native function") });
// Aborts the process today:
let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| engine.eval::<INT>("let t = 1; buggy(t)")));

The fix restores through the same defer! guard the script-function path in func_call.rs already uses, which also runs on unwind. The assertion is kept.

Test: test_native_panic_with_variable_first_argument_unwinds in tests/native.rs. buggy(t) is the aborting case and runs under every feature combination; the method-call spelling t.buggy() is a control and is gated on no_object, which disables the dot syntax (that was the CI failure on the no_object rows). Without the fix the test aborts with SIGABRT; with it the panic is caught and the engine is usable afterwards.

Checks: cargo test with default features, cargo test --test native under the three no_object rows of the CI matrix, cargo +nightly fmt --check. CHANGELOG line under 1.27.0's bug fixes, linking this PR.

🤖 Generated with Claude Code

schungx · 2026-10-17T00:13:43Z

Can you split this into two separate PRs just so we can track issues independently?

Also some CI's are failing.

Thanks for catching these!

schungx · 2026-10-17T02:06:34Z

 * Failed function resolutions are now cached properly for calls with other than two arguments - a regression from version 1.10 (thanks [`@ImTheSquid`](https://github.com/ImTheSquid) [`#1183`](https://github.com/rhaiscript/rhai/pull/1183)).
 * A function resolution cache is now created (if none exists) when entering a block. Previously, a loop body evaluated with no cache yet would resolve every function call again on each iteration (thanks [`@ImTheSquid`](https://github.com/ImTheSquid) [`#1183`](https://github.com/rhaiscript/rhai/pull/1183)).
 * Assigning a constant into a container variable (array, map etc.) now works properly without making the value read-only (thanks [`@cuervolu`](https://github.com/cuervolu) [`#1187`](https://github.com/rhaiscript/rhai/pull/1187)).
+* A panic inside a native Rust function that takes its first argument by value, called with a variable first argument, now unwinds normally and can be caught by `catch_unwind`. Previously the copied first argument was restored only on the normal-return path, so the unwind tripped an internal assertion and the process aborted ([`#1188`](https://github.com/rhaiscript/rhai/issues/1188)).


Better to link to this PR (#1189) than to the issue. Also good to add your username (similar style as others) as credit!

schungx · 2026-10-17T02:06:45Z

 * A function resolution cache is now created (if none exists) when entering a block. Previously, a loop body evaluated with no cache yet would resolve every function call again on each iteration (thanks [`@ImTheSquid`](https://github.com/ImTheSquid) [`#1183`](https://github.com/rhaiscript/rhai/pull/1183)).
 * Assigning a constant into a container variable (array, map etc.) now works properly without making the value read-only (thanks [`@cuervolu`](https://github.com/cuervolu) [`#1187`](https://github.com/rhaiscript/rhai/pull/1187)).
+* A panic inside a native Rust function that takes its first argument by value, called with a variable first argument, now unwinds normally and can be caught by `catch_unwind`. Previously the copied first argument was restored only on the normal-return path, so the unwind tripped an internal assertion and the process aborted ([`#1188`](https://github.com/rhaiscript/rhai/issues/1188)).
+* Under `sync`, a method call on a shared variable with a closure argument that captures the same variable now raises `ErrorDataRace`, as it does without `sync`, instead of panicking while resolving the function. `Dynamic::is_locked` now returns `true` under `sync` when the value cannot be read, which includes being locked for write by the current thread ([`#1188`](https://github.com/rhaiscript/rhai/issues/1188)).


yuvalrakavy · 2026-10-17T12:14:51Z

Done, thanks for the quick look.

Split: this PR now carries only the restore-on-unwind fix (one commit, force-pushed). The is_locked-under-sync change is Report a shared value locked by its own thread as locked under sync #1190.
CI: both failures were the new tests' scripts under feature combinations that cannot run them, not the fixes. t.buggy() needs the dot syntax, which no_object disables (MissingToken(";") at parse, so nothing panicked for catch_unwind to see); the aborting case buggy(t) now runs under every row and the method-call spelling is gated. The data-race test needs closures, so it is gated on no_closure too. Both verified locally under the exact rows that failed.
NoStdBuild: undefined symbol: __rustc::rust_eh_personality while linking no_std_test. It builds on both main and this branch with nightly 2026-10-09 here, the branch touches nothing no_std-specific, and main's own run of 2026-10-13 was green, so it looks like the newer nightly now expects the __rustc-namespaced personality symbol that no_std_test/src/main.rs defines under the old name. If it fails on main for you as well I am happy to send a one-line PR for the harness.
CHANGELOG: both lines now link their PR and carry the credit, in the same style as the others.

yuvalrakavy · 2026-10-17T14:26:37Z

NoStdBuild: it is the nightly, not this branch. rust-lang/rust#148413 (nightly 2026-10-16) mangles the personality symbol, so no_std_test's #[no_mangle] rust_eh_personality no longer links. Fix for the harness in #1191; this PR touches nothing no_std-specific.

schungx · 2026-10-17T14:33:07Z

There seem to be merge conflicts...

Can you resolve them? I'll merge if it can.

exec_native_fn_call swaps a copy of the first argument in through ArgBackup when a non-method native function is called with a variable first argument, and restored it only on the normal-return path. If the native function panicked, the unwind dropped the ArgBackup unrestored, its Drop assertion panicked during the unwind, and the process aborted, so no catch_unwind in the host could contain it. Restore through the same defer! guard the script-function path in this file already uses, which runs on unwind as well. The assertion is kept. The regression test runs the aborting case, `buggy(t)`, under every feature combination; the method-call spelling is a control and needs the dot syntax, which `no_object` disables, so it is gated. Fixes rhaiscript#1188 (the abort half).

yuvalrakavy · 2026-10-17T22:46:12Z

Rebased onto main (after #1190 and #1191); the CHANGELOG conflict is resolved with both lines kept. Thanks!

yuvalrakavy mentioned this pull request Oct 16, 2026

A panic inside a native function called with a variable first argument aborts the process (ArgBackup::drop asserts during unwinding) #1188

Closed

schungx added the bug label Oct 17, 2026

schungx linked an issue Oct 17, 2026 that may be closed by this pull request

A panic inside a native function called with a variable first argument aborts the process (ArgBackup::drop asserts during unwinding) #1188

Closed

schungx reviewed Oct 17, 2026

View reviewed changes

yuvalrakavy mentioned this pull request Oct 17, 2026

Report a shared value locked by its own thread as locked under sync #1190

Merged

yuvalrakavy force-pushed the fix-native-call-restore-on-unwind branch from de75c2b to 377df35 Compare October 17, 2026 12:14

yuvalrakavy changed the title ~~Restore a native call's copied first argument on unwind; report self-locked shared values under sync~~ Restore a native call's copied first argument on unwind Oct 17, 2026

yuvalrakavy mentioned this pull request Oct 17, 2026

no_std_test: declare the personality routine as the lang item (fixes NoStdBuild on current nightly) #1191

Merged

yuvalrakavy force-pushed the fix-native-call-restore-on-unwind branch from 377df35 to 5c8a405 Compare October 17, 2026 22:46

schungx merged commit aaf9090 into rhaiscript:main Oct 17, 2026
56 checks passed

BrewTestBot mentioned this pull request Oct 21, 2026

rhai 1.27.0 Homebrew/homebrew-core#320091

Merged

Navigation Menu

Repository navigation

Restore a native call's copied first argument on unwind - #1189

Restore a native call's copied first argument on unwind#1189
schungx merged 1 commit into
rhaiscript:mainfrom
yuvalrakavy:fix-native-call-restore-on-unwind

yuvalrakavy commented Oct 16, 2026 •

edited

Loading

Uh oh!

schungx commented Oct 17, 2026

Uh oh!

schungx Oct 17, 2026

Uh oh!

schungx Oct 17, 2026

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

schungx commented Oct 17, 2026

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

yuvalrakavy commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

schungx commented Oct 17, 2026

Uh oh!

schungx Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

schungx Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

schungx commented Oct 17, 2026

Uh oh!

yuvalrakavy commented Oct 17, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yuvalrakavy commented Oct 16, 2026 •

edited

Loading