Skip to content

Fix crash on component unload, catch panics at the FFI boundary, export SetPlatformCapabilities - #28

Open
Linklsk wants to merge 7 commits into
Sebekerga:mainfrom
Linklsk:fix/ffi-crash-and-safety
Open

Linklsk wants to merge 7 commits into
Sebekerga:mainfrom
Linklsk:fix/ffi-crash-and-safety

Conversation

@Linklsk

@Linklsk Linklsk commented Oct 7, 2026

Copy link
Copy Markdown

The crash

The component brings 1C down when the platform unloads it. Reproduced on 1C 8.5.4.1306, both
x86 and x86_64.

destroy_component reads the destructor pointer through a local ComponentWrapper which assumes
destroy sits right after the four interface pointers. In Component five fields are declared in
between, so the real offset was 96 on x86_64 and 48 on x86, while the wrapper read 32 and 16. That
slot holds memory_manager_ptr, which set_mem_manager fills with a live pointer to the platform
allocator — so DestroyObject called it as a function.

Windows Error Reporting matches this exactly: 1CV8C.exe, event type BEX, c0000005, DEP flag 8,
and StackHash in P4, meaning the faulting address belongs to no loaded module.

Pinned by a test rather than by inspection — this assertion fails on main:

assertion `left == right` failed
  left: 96      // actual offset of `destroy`
 right: 32      // where ComponentWrapper reads it

The fix moves destroy to the offset the wrapper expects and adds offset_of! assertions plus a
create → destroy roundtrip test, both running on x86_64 and i686.

Panics

Every platform callback except set_mem_manager now runs behind a guard. A panic in add-in code used
to unwind through the C++ frames of 1C and abort the process; now it is caught, reported through
AddError, and turned into the failure value the platform expects. The component is poisoned after
the first panic, since its state is unknown once the stack has unwound.

init also stores connection_ptr before running add-in code. The field existed but was never
populated anywhere, so the FFI layer had no way to reach AddError. This is what makes a panic inside
init itself reportable.

SetPlatformCapabilities

Now exported by extern_functions!. Per the Native API documentation, without this export the platform
withholds message output — so Connection::add_error and the whole MessageCode enum were dead code.
The pre-existing PLATFORM_CAPABILITIES static loses its mut, since references to static mut
become errors in edition 2024.

tVariant conversion

From<&TVariant> for ParamValue handled only Int32 and Double, silently turning VTYPE_I2,
VTYPE_UI1, VTYPE_UI2, VTYPE_ERROR and VTYPE_R4 into ParamValue::Empty. The docs note that
values arriving from the web client may come with any numeric type.

Taken from your feat/support-for-any-func-args-and-return

  • 96954c4 cherry-picked as is, with your authorship: os_string_nil did not append the terminator on
    unix while get_str reads names including it, so find_prop and find_method could never match on
    Linux. These fixes are independent of the Any feature, so they are split out here.
  • The lifetime elision in This::get_component, the explicit lifetime on ParamValues::iter, and
    #[allow(dead_code)] on name_override. They clear three of the four pre-existing clippy warnings.

Verification

  • cargo test --workspace — 84 passed, 0 failed
  • cargo test -p native_api_1c_core --target i686-pc-windows-msvc — 15 passed, so the layout
    assertions run on 32-bit too
  • cargo clippy --workspace --all-targets — one pre-existing warning left (string_utils.rs:42)
  • cargo fmt --all -- --check — clean. Note that find_method.rs and find_prop.rs still make rustfmt
    abort with an internal left behind trailing whitespace error, which predates this branch
  • Checked end to end in 1C 8.5.4.1306 on both x86 and x86_64: the component loads, properties and
    methods work, and a deliberate panic is reported into the message window instead of killing 1C

Left alone

  • dtor is 0 in all four vtables. If the platform ever calls the virtual destructor instead of
    DestroyObject, that is a jump to null with the same signature as the bug fixed here.
  • A consumer building with panic = "abort" disables the guard entirely — noted in the README.
  • No embedded manifest, and the CRT is linked dynamically (VCRUNTIME140.dll), while the 1C docs ask
    for non-system runtimes to be linked statically and for a manifest in Windows components.

Linklsk and others added 7 commits October 7, 2026 13:24
From<&TVariant> only handled Int32 and Double, silently turning VTYPE_I2, VTYPE_UI1, VTYPE_UI2, VTYPE_ERROR and VTYPE_R4 into ParamValue::Empty. Per the Native API docs the platform stores integers in lVal and fractions in dblVal regardless of the tag, so those are read now. Adds unit tests covering every tag.
Taken from upstream feat/support-for-any-func-args-and-return, where they accompany the Any feature but are independent of it: elide the lifetime on This::get_component, make ParamValues::iter lifetime explicit, and allow the unused name_override field. Clears three of the four pre-existing clippy warnings. Also corrects the os_string_nil doc comment on unix, which still claimed there is no terminator after the previous commit added one.
ComponentWrapper assumes destroy sits right after the four interface pointers, but in Component five fields were declared in between, so destroy landed at offset 96 on x86_64 and 48 on x86 while the wrapper read offset 32 and 16. That slot holds memory_manager_ptr, which set_mem_manager fills with a live pointer to the platform allocator, so DestroyObject called it as a function. 1C died with c0000005 and DEP flag 8 at an address belonging to no loaded module.

Move destroy up to the offset the wrapper expects and pin the layout with offset_of! assertions plus a create/destroy roundtrip test that runs on both x86_64 and i686.
A panic in add-in code used to unwind through the C++ frames of the platform, which aborts the process. Every callback in init_done, lang_extender, locale_base and user_lang_base now runs through a guard that catches it, reports it with AddError and returns the failure value the platform expects. set_mem_manager is left alone: it only copies a pointer.

The add-in state is unknown after unwinding, so the component is poisoned on the first panic and every later call returns the fallback without touching it.

init now stores connection_ptr before running add-in code. The field existed but was never populated, so the FFI layer had no way to reach AddError - which is also what makes a panic inside init itself reportable.

Adds AppCapabilities for the SetPlatformCapabilities negotiation and tests for the guard, the poisoning and the negotiation.
The platform withholds message output and platform info from components that do not negotiate capabilities, so Connection::add_error was a silent no-op. extern_functions! already generated a PLATFORM_CAPABILITIES static for this but never emitted the function that fills it.

The static loses its mut: AtomicI32 needs no unsafe access, and references to static mut become errors in edition 2024. Like the other four exports it uses extern C, which the x86 build confirms is the right convention for the DLL exports, while the vtable methods stay extern system.
ReportMessage pushes a line into the 1C message window through add_error, which only reaches the user now that capabilities are negotiated. Panic raises deliberately so the guard can be checked end to end; it poisons the component, so it has to be called last.

README documents the guard, the poisoning and the panic = abort caveat.
@Linklsk Linklsk closed this Oct 7, 2026
@Linklsk Linklsk reopened this Oct 15, 2026
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