Make EventLoopProxy Sync

[Liamolucko]

• Tested on all platforms changed
• Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

Previously, EventLoopProxy was Send but not Sync. The only reason for this that I could see was that mpsc::Sender wasn't Sync; however, the alternative mpsc::SyncSender is, so I've switched to using that instead.

It's not named that because it implements Sync; the difference is that SyncSender has a fixed-size buffer and will block if its buffer is full, whereas Sender will never block. For the size of the buffer, I've just arbitrarily picked 10; I've got no idea what would be optimal.

The reason I think it needs to implement Sync is for the sake of #1199. In an executor which polls futures within the event loop, the best way I can think of to implement Waker is by sending an event through EventLoopProxy to wake up the event loop; however, Waker is Send and Sync, so EventLoopProxy also has to be Send and Sync for a Waker implementation to use it.

On the web, EventLoopProxy is neither Send nor Sync. This could be fixed, but would be much more complicated and isn't needed for the async use case - on the web, wasm_bindgen_futures::spawn_local could just be used instead, so a custom Waker implementation isn't needed.

[madsmtm]

I agree with this in principle (haven't looked into the implementation yet), however I would like our platforms to not diverge further on what marker traits they implement (since that is a quite hard-to-debug thing).

So maybe we could fix the "web's EventLoopProxy isn't Send/Sync" while we're at it? If not, I don't think EventLoopProxy should be those marker traits on any platform!

[Liamolucko]

There are two ways I can think of to implement Send + Sync for EventLoopProxy on the web:

• Pull in wasm-bindgen-futures and some async channel library as dependencies, and spawn a task which just waits for incoming EventLoopProxy messages and handles them. This is easier, but pulls in a couple extra dependencies.
• Manually implement a thread waker using atomics, and combine that with a std channel to send messages. This would basically mean duplicating the thread waker that wasm-bindgen-futures uses internally.

Which would you prefer?

[Liamolucko]

Okay, I've now implemented Send + Sync on the web as well using wasm-bindgen-futures and async-channel.

[maroider]

I'd honestly prefer the "manual" approach, but I don't know how much more work that would be. In either case, the web backend is kind of weird to me in many ways, so I think I'll have to read more of it and then probably consult someone before I feel comfortable with either approach.

[Liamolucko]

Using SyncSender here is a bit more problematic than I initially thought - if its buffer is full, calling EventLoopProxy::send_event will deadlock, which would be quite frustrating to debug.

Also, this isn't as vital for async as I thought - wrapping the EventLoopProxy in a mutex makes it Sync, so EventLoopProxy itself doesn't have to be.

[daxpedda]

I just stumbled on the same issue, but I only need Send, not Sync, to implement a multi-threaded engine on Wasm that can send UserEvents to the Winit loop.

I would be more then happy to contribute, what would be the current blocker?

[Liamolucko]

I would be more then happy to contribute, what would be the current blocker?

The main problem, at least in my opinion, was that using SyncSender on native platforms meant that calling EventLoopProxy::send_event on the main thread would deadlock if the buffer was full.

That's not an issue with the implementation on the web though, so I don't think there'd be any major problems if you copy-pasted that bit into a new PR. You might want to manually make it not Sync though, for consistency with native platforms.

[daxpedda]

I will make a new PR only for Send then, with the goal of having EventLoopProxy share the same traits on all platforms.

[daxpedda]

This can be closed in favor of #2834.
Ultimately if we want to implement Sync for EventLoopProxy, we need to do it for all backends, similar to #2749.

We had a brief talk about this on Matrix and decided that this can be done by the user by just wrapping it around a Mutex, which we would have to do internally anyway.

This would also be fixed automatically by rust-lang/rust#111087.

[madsmtm]

Noting that this has now been fully resolved by #3449.