smol-rs / event-listener

Notify async tasks or threads
Apache License 2.0
449 stars 30 forks source link

feat: Add a loom implementation for event-listener #126

Closed jbr closed 8 months ago

jbr commented 8 months ago

The loom tests aren't particularly useful yet, but the crate compiles and runs under loom now.

Note: This crate does not currently compile under loom without std because concurrent-queue does not compile with that combination (can't find spin_loop).

closes #23

notgull commented 8 months ago

I added Loom to the CI in https://github.com/smol-rs/concurrent-queue/pull/62. It looks like it works. Maybe it's the optional feature flag that's gumming it up.

notgull commented 8 months ago

I think crossbeam does this right. See here:

https://github.com/crossbeam-rs/crossbeam/blob/9e8596105bc9a6b343918b6ad1c9656dc24dc4f9/crossbeam-epoch/Cargo.toml#L30-L34

and here:

https://github.com/crossbeam-rs/crossbeam/blob/9e8596105bc9a6b343918b6ad1c9656dc24dc4f9/crossbeam-epoch/Cargo.toml#L39-L44

jbr commented 8 months ago

I'm surprised concurrent-queue Loom tests are failing, but I guess that's due to loom not being checked in CI there.

To be clear, it's not that the concurrent-queue loom tests are failing, or even that concurrent-queue can't build under loom. This PR for event listener correctly builds concurrent-queue under loom and runs tests against concurrent-queue loom.

The issue is this specific scenario for concurrent-queue, which may or may not even matter: RUSTFLAGS="--cfg=loom" cargo build --no-default-features --features loom (in concurrent-queue)