wasmtime / PR #11836 feat(no_std): Add custom sync primit... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11836 feat(no_std): Add custom sync primit...

Wasmtime GitHub notifications bot (Oct 11 2025 at 00:44):

salmans opened PR #11836 from salmans:custom-sync to bytecodealliance:main:

This PR introduces support for custom synchronization primitives in wasmtime's custom platform backend, enabling embedders to provide their own mutex and rwlock implementations through C FFI.

Motivation

The existing implementation in sync_nostd.rs uses uncontended locks that panic on contention, making it unsuitable for multithreaded environments. However, many no_std environments (such as OS kernels, embedded RTOS, or bare-metal systems with threading support) do have access to threading primitives and need proper synchronization.

This implementation follows the approach suggested in the comments sync_nostd.rs implementation, which recommended that hosts provide their own synchronization primitives via the C API for truly multithreaded no_std environments.

Changes

- New custom-sync-primitives feature: Allows wasmtime to use host-provided synchronization primitives instead of uncontended locks in sync_nostd.rs
- C API declarations (in capi.rs): Defines the FFI interface for sync lock operations:
- wasmtime_sync_lock_new/free - Mutex lifecycle management
- wasmtime_sync_lock_acquire/release - Exclusive locking
- wasmtime_sync_rwlock_read/write - Reader-writer lock operations
- wasmtime_sync_rwlock_read_release/write_release - Lock release
- Implementation (in custom/sync.rs): Rust wrappers that call the C API functions, implementing wasmtime's internal OnceLock and RwLock types using the custom primitives

Wasmtime GitHub notifications bot (Oct 11 2025 at 00:58):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 11 2025 at 01:11):

salmans has marked PR #11836 as ready for review.

Wasmtime GitHub notifications bot (Oct 11 2025 at 01:11):

salmans requested fitzgen for a review on PR #11836.

Wasmtime GitHub notifications bot (Oct 11 2025 at 01:11):

salmans requested wasmtime-core-reviewers for a review on PR #11836.

Wasmtime GitHub notifications bot (Oct 12 2025 at 15:47):

salmans edited PR #11836:

This PR introduces support for custom synchronization primitives in wasmtime's custom platform backend, enabling embedders to provide their own mutex and rwlock implementations through C FFI.

Motivation

The existing implementation in sync_nostd.rs uses uncontended locks that panic on contention, making it unsuitable for multithreaded environments. However, many no_std environments (such as OS kernels, embedded RTOS, or bare-metal systems with threading support) do have access to threading primitives and need proper synchronization.

This implementation follows the approach suggested in the comments in sync_nostd.rs implementation, which recommended that hosts provide their own synchronization primitives via the C API for truly multithreaded no_std environments.

Changes

- New custom-sync-primitives feature: Allows wasmtime to use host-provided synchronization primitives instead of uncontended locks in sync_nostd.rs
- C API declarations (in capi.rs): Defines the FFI interface for sync lock operations:
- wasmtime_sync_lock_new/free - Mutex lifecycle management
- wasmtime_sync_lock_acquire/release - Exclusive locking
- wasmtime_sync_rwlock_read/write - Reader-writer lock operations
- wasmtime_sync_rwlock_read_release/write_release - Lock release
- Implementation (in custom/sync.rs): Rust wrappers that call the C API functions, implementing wasmtime's internal OnceLock and RwLock types using the custom primitives

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton submitted PR review:

Thanks for this! I've got some various ideas/suggestions below I'm curiuos to get your take on. The high-level goal would be to simplify this module by putting a little bit more complexity into the implementation-side of the symbols and to additionally reduce duplication with the sync_nostd module since this is pretty tricky code to get right.

This is all primarily motivated by the lack of tests in this PR and the overall difficulty of testing an implementation. Ideally, if you're up for it, the examples/min-platform example would be updated with a new feature to implement synchronization primitives in terms of pthread_* functions. Would you be up for adding such a test?

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

One drawback of this current approach is that this module has a lot of nontrivial implementation logic which isn't tested at all in our CI. Adding testing would be somewhat difficult unfortunately, as well. Given that I might recommend a few changes to the C API to make loops like this simpler:

wasmtime_sync_lock_new would take a *mut usize parameter rather than returning a usize (similar to all other functions).

It would be an API contract of wasmtime_sync_lock_new that the initial value of the pointer passed in is 0. That way if the implementation uses 0 to mean "initialized but inert" the function would be a noop. Otherwise it's up to the implementation to initialize-from-zero.

Lazy initialization would be deferred to the implementor rather than implemented here. That would remove the spin_loop() which is generally not appropriate and is best to leave to the actual synchronization library which can probably do something more productive than spinning (e.g. by yielding)

Overall that would remove the need for this function entirely and should greatly simplify this interface.

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

For this the main purpose of these custom cfgs are for when there's custom logic behind the definition, such as a cascade of other cfg's or checking other conditions. For this one it's an alias of feature = "custom-sync-primitives" so I think it's fine to omit.

Now that being said in the code this cfg is always coupled with not(feature = "std") as well. Given that I think it would be reasonable to keep has_custom_sync as a custom cfg, but factor in the not(feature = "std") calculating to enabling it as well. Basically when feature = "std" is enabled this has_custom_sync would never be enabled.

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

In the interest of simplifying this module as much as possible due to the difficulty to test this, I think it would be good to share this implementation with the preexisting code in sync_nostd.rs which I suspect this is copied from. Could the sync_nostd module have a single RwLock, but internally there'd be something like a "raw" rwlock which has no data and is just the lock: HostLock primitive here? That way without custom-sync-primitives it'd use the panic-on-contention implementation from today but with the feature it'd use a lock defined in terms of the C API.

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

Could the panic-on-contention error messages of this module be updated to mention the custom-sync-primitives Cargo feature?

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

What do you think about using a custom constructor/destructor for an RwLock rather than sharing the same ones for the mutex? That way it leaves implementations the ability to have distinct implementations of the two, and if they're the same it wouldn't be too hard to route them to the same implementation.

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

This'll want to be tagged as an unsafe function because it's generally not safe to call. Although this could also perhaps be exclusively part of the Drop implementation?

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton created PR review comment:

This makes me realize that this is actually missing from the sync_nostd implementation today. This is another point in favor for me to reduce the duplication between the two modules.

Wasmtime GitHub notifications bot (Oct 13 2025 at 14:57):

alexcrichton requested alexcrichton for a review on PR #11836.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:27):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:28):

salmans created PR review comment:

I agree. Changes are reflected.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:28):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:30):

salmans created PR review comment:

That's a great idea. I made those changes.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:30):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:31):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:31):

salmans created PR review comment:

I unified the two implementations but wondering if that properly reflects your ideas. Please let me know if something doesn't seem right or missing.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:31):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:31):

salmans created PR review comment:

Done!

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:32):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:32):

salmans created PR review comment:

Done!

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:33):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:33):

salmans created PR review comment:

Wondering if this is a good idea.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:47):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:50):

salmans commented on PR #11836:

@alexcrichton Thanks for the quick review. I agree. with all points you made and tried to address them. And absolutely, I'll extend the min_platform example accordingly (it's a very useful example that has helped me a lot in the past!).

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:50):

salmans edited a comment on PR #11836:

@alexcrichton Thanks for the quick review. I agree with all points you made and tried to address them. And absolutely, I'll extend the min_platform example accordingly (it's a very useful example that has helped me a lot in the past!).

Wasmtime GitHub notifications bot (Oct 13 2025 at 22:56):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 14 2025 at 00:47):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 14 2025 at 00:51):

salmans commented on PR #11836:

I pushed two more commits to reflect the custom sync changes in min-platform example. The first one implements the sync primitives but to have a truly multithreaded system, I needed to change the existing TLS implementation that assumes a single threaded execution. Feel free to drop the second commit if not needed or let me know if needs adjustment.

Wasmtime GitHub notifications bot (Oct 14 2025 at 00:55):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

For this I'd say it's reasonable to unconditionally call wasmtime_sync_lock_free and leave it up to the implementation to be a noop if the lock was never actually used.

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

If possible let's keep this removed because that means that cargo test will still test this module's internals and run the #[cfg(test)] module, otherwise the tests won't run at all.

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

I might recommend dropping this field in the guards and accessing it directly from self.lock to shrink the size of the guard

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

Hm ok on second though I don't think I fully thought this through. Fully thinking this through now, I think let's actually delete the *_new functions entirely. Any lazy initialization, if necessary, can be done by wasmtime_sync_rwlock_write here for example. The C API would then have two documented guarantees:

Freshly initialized locks will always have a zero pattern for initialization.

*_free may be called on a lock that was initialized but never used (e.g. is a zero pattern)

That still allows for lazy init and means that wasmtime never explicitly calls *_new and defers initialization, if any, to the implementation.

Basically otherwise it feels a bit odd calling *_new on each operation. Apologies for the confusion!

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

I don't think that this is a valid implementation of the API because if locks are acquired in order then this could cause a deadlock by having multiple locks get mapped to the same lock. I think for the sample implementation here heap allocation will be needed.

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

Dropping the lock here will want to happen as part of a Drop-based destructor instead of manually here to handle panicking, for example, in the constructor

Wasmtime GitHub notifications bot (Oct 14 2025 at 14:47):

alexcrichton created PR review comment:

Also I think it'd help cut down on some duplication by placing this Drop implementation on HostLock above to avoid having it both here and for Mutex<T>

Wasmtime GitHub notifications bot (Oct 14 2025 at 22:40):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 14 2025 at 22:53):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 14 2025 at 23:11):

salmans submitted PR review.

Wasmtime GitHub notifications bot (Oct 14 2025 at 23:11):

salmans created PR review comment:

Good catch, thank you!
I pushed a couple of new commits, addressing your previous comments as well as this one. The example is now using heap allocation but I guess it's fine to use stdlib here. Also when running tests locally, I discovered a race condition (when running ~500 threads concurrently), so added CAS logic, which fix the issue.

Wasmtime GitHub notifications bot (Oct 15 2025 at 06:11):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 15 2025 at 14:53):

alexcrichton submitted PR review:

Thanks again for your work on this!

Wasmtime GitHub notifications bot (Oct 15 2025 at 15:09):

salmans commented on PR #11836:

Thanks so much for your feedback and the quick turnaround!

Wasmtime GitHub notifications bot (Oct 16 2025 at 14:27):

salmans commented on PR #11836:

@alexcrichton this apparently got kicked out of the merge queue. Do you mind trying one more time? Thanks again!

Wasmtime GitHub notifications bot (Oct 16 2025 at 14:31):

alexcrichton commented on PR #11836:

Oh that's because of the test failure that happened. I think there's a different version of bindgen installed in CI than you were running locally, and it's fine to either update CI or downgrade your version locally, whichever's easiest for you

Wasmtime GitHub notifications bot (Oct 16 2025 at 16:20):

salmans requested fitzgen for a review on PR #11836.

Wasmtime GitHub notifications bot (Oct 16 2025 at 16:20):

salmans requested wasmtime-default-reviewers for a review on PR #11836.

Wasmtime GitHub notifications bot (Oct 16 2025 at 16:20):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 16 2025 at 16:44):

salmans commented on PR #11836:

Ah thank you! I updated the CI version. It should be good now.

Wasmtime GitHub notifications bot (Oct 16 2025 at 17:26):

salmans commented on PR #11836:

This got bounced again but I'm not able to see any logs.

Wasmtime GitHub notifications bot (Oct 16 2025 at 17:27):

salmans deleted a comment on PR #11836:

This got bounced again but I'm not able to see any logs.

Wasmtime GitHub notifications bot (Oct 16 2025 at 17:32):

salmans updated PR #11836.

Wasmtime GitHub notifications bot (Oct 16 2025 at 17:34):

salmans commented on PR #11836:

I had accidentally edited the comments in the header file generated by cbindgen, which caused the PR get rejected again (sorry about that!). It should be fine now.

Wasmtime GitHub notifications bot (Oct 16 2025 at 19:24):

alexcrichton merged PR #11836.

Last updated: Dec 06 2025 at 07:03 UTC