parking_lot_core 0.8.1 wasm link issue

[emilk]

Starting with parking_lot_core 0.8.1, when compiling to wasm there is now a depdendency on "env".

This means one can no longer use parking_lot from within a wasm runtime, such as wasmtime.

When compiling for the web, one can address this problem with parking_lot = { version = "0.11", features = ["wasm-bindgen"] }, but this does not work for non-web wasm runtimes.

Reproduce

Cargo.toml:

[package]
name = "parking_lot_test"
version = "0.1.0"

[dependencies]
parking_lot = "0.11"
parking_lot_core = "=0.8.1"

main.rs:

fn main() {
    let m = parking_lot::Mutex::new(());
    let _lock = m.lock();
}

Using wasm2wat:

§ cargo build --target "wasm32-unknown-unknown"
§ wasm2wat target/wasm32-unknown-unknown/debug/parking_lot_test.wasm | rg import
> (import "env" "now" (func $now (type 7)))

(with parking_lot_core = "=0.8.0" there is no import "env")

[bjorn3]

https://github.com/Amanieu/parking_lot/compare/0.8.0...0.8.1?w=1#diff-f8a0a8b9353ddc2e1efd135d75ec0668f64e2674c15e9b1cda089cbd69f60372 shows no difference in the parking implementation between v0.8.0 and v0.8.1 that could explain this. Does this still happen with LTO enabled?

[emilk]

I tested it now with --release and [profile.release] lto = true – same problem.

[emilk]

I suspect this is related to parking_lot_core:s use of https://github.com/sebcrozet/instant

Does parking_lot require knowing the current time? Because then it is fundamentally incompatible with "pure" wasm

[bjorn3]

The parking_lot algorithm itself need to know the current time for fair locks but the single-threaded wasm option shouldn't need this as there are no other threads against which it needs to be fair.

[emilk]

Interesting!

Is there a way to tell parking_lot that it is compiling for a single-threaded target, so that it does not require the instant crate at all? If not, should there be?

[bjorn3]

It should compile for a single-threaded target by default. Multi-threading requires the bulk memory wasm proposal and atomic instructions. Both of which are not supported for the WASM MVP that is targeted by default. In addition it requires all parts of the program to be compiled with multi-threading support I believe, but libstd isn't by default.

[emilk]

I suspect that the only reason parking_lot 0.8.0 worked and 0.8.1 doesn't is because some subtle failure of rustc to elide unused code (using instant).

---

For my use-case ("pure wasm" = no instant) we would need cfg-blocks around all uses of the instant crate, as well as around functions that uses time, such as wait_until, try_lock_until etc.

For example:

#[cfg(feature = "instant")]
unsafe fn park_until(&self, timeout: Instant) -> bool { ... }

This would be a very intrusive change to the parking_lot code base, disabling all uses of times (Instant and Duration).

(Using #[cfg(not(target_arch = "wasm32"))] would not be a good idea, because when compiling wasm to the web instant works fine with features = ["wasm-bindgen"]).

---

Another alternative is that I create a new wrapper crate around parking_lot which exposes only what I need (Mutex and RwLock) and that has a feature-flag for single-threaded, no-instant compile-targets that just uses a bool to check if a Mutex is already locked (just to panic on reentrant double-lock).

This solution is not very attractive though, as it would require third-party-dependencies to also switch to my new wrapper crate.

Ideally parking_lot would work in a pure wasm (no instant) environment (at least with some special flag)

[faern]

Why do you need locking primitives in a single threaded environment?

[emilk]

Why do you need locking primitives in a single threaded environment?

Great question!

Say you have a library that may or may not be used from a multithreaded context, so the library uses parking_lot::Mutex in a few places. There is also an app that wants to use that library. The app happens to be compiled to a singlethreaded wasm32 target. Should this be possible?

The answer is: it used to be possible, but with parking_lot_core 0.8.1 it no longer is.

So who's responsibility is it to fix this?

The app can do a work-around by addingparking_lot_core = "=0.8.1" to it's Cargo.toml, but that work-around will only work for so long. As soon as the library upgrades to a future `parking_lot_core = 0.9.0" then the app can no use the latest version of that library.

The library could add feature flags around all uses of parking_lot, but there are a lot of libraries using parking_lot, and the app developer will have trouble changing all the libraries.

The third place to fix this is in parking_lot itself, by stating "parking_lot supports pure wasm32 contexts". I think this is the route that is the best.

---

In summary:

Either we say "parking_lot should work for pure wasm32 targets" and work towards that, OR we say "if you use parking_lot in your library, then your library cannot be used by apps that want to compile to pure wasm32 targets".

[faern]

That makes sense. I got the imoression that your code needed it directly and your code was targeting single threaded wasm only :)

[ibaryshnikov]

Isn't it related to this?
rust-lang/rust#72758 (comment)

(import "env" "now" (func $now (type 7)))
it also feels like coming from this line

if let Some(left) = timeout.checked_duration_since(Instant::now()) {

[bjorn3]

That one is only used when wasm atomics are enabled, which aren't by default.