Attempt to prevent crashes during exit due to deregistering frames on exit #5893

zebreus · 2025-11-20T16:20:21Z

This PR ensures that multiple threads don't access the functions for registering and deregistering frames at the same time, which can lead to crashes, as the functions are not guaranteed to be thread-safe (at least I couldn't find any documentation stating they were).

It also no longer deregisters frames during Drop, which caused issues when exiting the wasmer. Instead, deregistrations are now queued in a GLOBAL_UNREGISTRY and processed before the next batch of registrations.

Fixes #5877

… exit

zebreus · 2025-11-20T16:22:41Z

Testing this is a bit dumb. I ran while target/debug/wasmer --disable-cache cowsay aa --registry=wasmer.io ; do : ; done for 10 minutes, and it didn't crash. Before this PR it would usually abort after 5-10 runs.

Copilot

Pull Request Overview

This PR addresses thread safety issues during frame registration/deregistration by introducing a global mutex-protected registry. The key change is deferring frame deregistration from the Drop implementation to avoid crashes during program shutdown and concurrent access.

Key Changes:

Introduces GLOBAL_UNREGISTRY mutex to serialize frame registration/deregistration operations
Defers actual deregistration by queuing frames in Drop instead of immediately calling deregister functions
Processes queued deregistrations before each new registration batch

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/compiler/src/engine/unwind/systemv.rs

Copilot · 2025-11-20T16:24:06Z

lib/compiler/src/engine/unwind/systemv.rs

+                self.compact_unwind_mgr.deregister();
+            }


The self.compact_unwind_mgr.deregister() call is inside the loop but appears to be called on the same manager object for each item in the unregistry. This means if there are N registrations in the unregistry, deregister() will be called N times on the same manager. This seems incorrect - it should likely be called once after the loop, similar to how self.compact_unwind_mgr.register() is called once in the publish() method (line 143). Consider moving this call outside the loop.

Suggested change

self.compact_unwind_mgr.deregister();

}

}

self.compact_unwind_mgr.deregister();

Copilot · 2025-11-20T16:24:07Z

lib/compiler/src/engine/unwind/systemv.rs

            // TODO: I am not 100% sure if it's correct to never deregister the `.eh_frame` section. It was this way before
            // I started working on this, so I kept it that way.
            unsafe {
                compact_unwind::__unw_add_dynamic_eh_frame_section(eh_frame.as_ptr() as usize);


On macOS aarch64, the code registers frames via __unw_add_dynamic_eh_frame_section at line 170, but unlike the non-macOS path (line 245), it doesn't add the registration to self.registrations. This means when Drop is called (line 294), self.registrations will be empty on macOS, so nothing gets queued in the unregistry for later deregistration. This could lead to the registered frames never being deregistered. Consider adding the registration to self.registrations after line 170, similar to how it's done at line 245 for non-macOS platforms.

Suggested change

compact_unwind::__unw_add_dynamic_eh_frame_section(eh_frame.as_ptr() as usize);

let registration = eh_frame.as_ptr() as usize;

compact_unwind::__unw_add_dynamic_eh_frame_section(registration);

self.registrations.push(registration);

lib/compiler/src/engine/unwind/systemv.rs

zebreus · 2025-11-21T09:57:37Z

As the code stands before this PR, we never unregister frames on macOS aarch64. This is caused by us only calling deregister on the CompactUnwindManager if there are entries in registrations, but the macOS aarch64 path never adds entries to that.

marxin

What a smart idea @zebreus, haven't thought about it!

Testing this is a bit dumb. I ran while target/debug/wasmer --disable-cache cowsay aa --registry=wasmer.io ; do : ; done for 10 minutes, and it didn't crash. Before this PR it would usually abort after 5-10 runs.

Yeah, I was basically doing the very same, don't have anything better. I've got an idea how to improvement the implementation, let me do it in the inline comments.

marxin · 2025-11-21T10:11:48Z

lib/compiler/src/engine/unwind/systemv.rs

 use crate::types::unwind::CompiledFunctionUnwindInfoReference;

 /// Represents a registry of function unwind information for System V ABI.
 pub struct UnwindRegistry {


What about splitting the type into 2: UnpublishedUnwindRegistry and PublishedUnwindRegistry?

marxin · 2025-11-21T10:12:17Z

lib/compiler/src/engine/unwind/systemv.rs

+#[cfg(not(all(target_os = "macos", target_arch = "aarch64")))]
+static GLOBAL_UNREGISTRY: Mutex<Vec<usize>> = Mutex::new(Vec::new());
+#[cfg(all(target_os = "macos", target_arch = "aarch64"))]
+static GLOBAL_UNREGISTRY: Mutex<(Vec<usize>, Vec<compact_unwind::CompactUnwindManager>)> =


This could be then unconditional on target: Mutex<Vec<PublishedUnwindRegistry>>.

marxin · 2025-11-21T10:12:47Z

lib/compiler/src/engine/unwind/systemv.rs

        #[cfg(all(target_os = "macos", target_arch = "aarch64"))]
        {
+            // Acquire the unregistry lock to avoid interleaved registrations/deregistrations.
+            let mut unregistry = GLOBAL_UNREGISTRY.lock().unwrap();


Here we would iterate the PublishedUnwindRegistry, and call PublishedUnwindRegistry::unregister?

marxin · 2025-11-21T10:13:25Z

lib/compiler/src/engine/unwind/systemv.rs

    fn drop(&mut self) {
        if self.published {
-            unsafe {
+            let mut unregistry = GLOBAL_UNREGISTRY.lock().unwrap();


and this will simply push to the GLOBAL_UNREGISTRY vector.

zebreus · 2025-11-21T11:56:12Z

However, the code in this PR causes segfaults in the tests. I can't reproduce it outside of running the whole test suite, but it happens at https://github.com/gcc-mirror/gcc/blob/master/libgcc/unwind-dw2-fde.c#L310, so I assume the pointer is invalid by the time we unregister it.

Which makes sense, as the pointer effectively points into the &[u8] passed to the register frame. Technically, the previous implementation was also unsafe, since that buffer is not guaranteed to remain alive while the drop is executing. That may even be the original issue.

zebreus · 2025-11-21T12:10:29Z

Copying the eh_frame into a leaked heap allocation in register_frame solves the segfault introduced by my fix.

zebreus · 2025-11-26T09:44:17Z

I dont really know a good way forward. My best guess would be to restore the previous behaviour of deregistering in drop and add a handler into atexit on linux to set a global atomic flag. And check that global before deregistering. Not sure if the order of that works out, we need to test this well.

marxin · 2025-11-26T10:42:09Z

Alternative approach #5912.

Attempt to prevent crashes during exit due to deregistering frames on…

842b0a0

… exit

zebreus requested review from Copilot and marxin November 20, 2025 16:20

zebreus requested a review from syrusakbary as a code owner November 20, 2025 16:20

Copilot started reviewing on behalf of zebreus November 20, 2025 16:21 View session

Copilot finished reviewing on behalf of zebreus November 20, 2025 16:23

Copilot AI reviewed Nov 20, 2025

View reviewed changes

zebreus added 2 commits November 21, 2025 10:06

Fix copilot review comments

acb6f4b

Attempt to also defer the compact unwind manager on macos

f7b6d14

marxin approved these changes Nov 21, 2025

View reviewed changes

marxin reviewed Nov 21, 2025

View reviewed changes

marxin mentioned this pull request Nov 25, 2025

Fix frame registration/deregistration for libgcc and libunwind #5886

Closed

-                compact_unwind::__unw_add_dynamic_eh_frame_section(eh_frame.as_ptr() as usize);
+                let registration = eh_frame.as_ptr() as usize;
+                compact_unwind::__unw_add_dynamic_eh_frame_section(registration);
+                self.registrations.push(registration);

Attempt to prevent crashes during exit due to deregistering frames on exit #5893

Are you sure you want to change the base?

Attempt to prevent crashes during exit due to deregistering frames on exit #5893

Conversation

zebreus commented Nov 20, 2025

Uh oh!

zebreus commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zebreus commented Nov 21, 2025

Uh oh!

marxin left a comment

Choose a reason for hiding this comment

Uh oh!

marxin Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

marxin Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

marxin Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

marxin Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

zebreus commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zebreus commented Nov 21, 2025

Uh oh!

zebreus commented Nov 26, 2025

Uh oh!

marxin commented Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zebreus commented Nov 20, 2025 •

edited

Loading

zebreus commented Nov 21, 2025 •

edited

Loading