macos: `*mut c_void` cannot be sent between threads safe

[hulxv]

Description

In Citadel-tech, we are working on an event-loop library, and it works without issues on Linux and Windows, but for some reason, it fails on MacOS (or kqueue generally, as I think). That happens because one of the Event attributes couldn't be sent safely between threads (See the opened issue about that citadel-tech/Event-Loop#68).

error message:

*mut c_void` cannot be sent between threads safely
    --> src/reactor.rs:73:24
     |
  73 |           self.pool.exec(move || {
     |                     ---- ^------
     |                     |    |
     |  ___________________|____within this `{closure@src/reactor.rs:73:24: 73:31}`
     | |                   |
     | |                   required by a bound introduced by this call
  74 | |             let entry = registry.get(&token);
  75 | |
  76 | |             if let Some(entry) = entry {
...    |
  85 | |         })
     | |_________^ `*mut c_void` cannot be sent between threads safely
     |
     = help: within `{closure@src/reactor.rs:73:24: 73:31}`, the trait `Send` is not implemented for `*mut c_void`
note: required because it appears within the type `libc::unix::bsd::apple::kevent`
    --> /Users/0xegao/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/libc-0.2.177/src/unix/bsd/apple/mod.rs:1322:16
     |
1322 |     pub struct kevent {
     |                ^^^^^^
note: required because it appears within the type `Event`
    --> /Users/0xegao/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/mio-1.0.4/src/event/event.rs:17:12
     |
  17 | pub struct Event {
     |            ^^^^^
note: required because it's used within this closure
    --> src/reactor.rs:73:24
     |
  73 |         self.pool.exec(move || {
     |                        ^^^^^^^
note: required by a bound in `ThreadPool::exec`
    --> src/thread_pool.rs:49:23
     |
  47 |     pub fn exec<F>(&self, task: F) -> Result<()>
     |            ---- required by a bound in this associated function
  48 |     where
  49 |         F: FnOnce() + Send + 'static,
     |                       ^^^^ required by this bound in `ThreadPool::exec`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `mill-io` (lib) due to 1 previous error

this happens here:

https://github.com/citadel-tech/Event-Loop/blob/16d70c6f524f6bde80bbbc42dd2209edb94de93f/src/reactor.rs#L68-L86

There are some solutions for our case, like making a safe wrapper that contains the important information, but I want to know if there are any solutions that can fix it internally in mio-rs

[Thomasdezeeuw]

Event is thread safe, including when using kqueue. We have the code to make Events thread safe:

mio/src/sys/unix/selector/kqueue.rs

Lines 356 to 362 in c9831c7

	// `Events` cannot derive `Send` or `Sync` because of the
	// `udata: *mut ::c_void` field in `libc::kevent`. However, `Events`'s public
	// API treats the `udata` field as a `uintptr_t` which is `Send`. `Sync` is
	// safe because with a `events: &Events` value, the only access to the `udata`
	// field is through `fn token(event: &Event)` which cannot mutate the field.
	unsafe impl Send for Events {}
	unsafe impl Sync for Events {}

But for Event it will a little harder as it's a type alias for libc::kevent:

mio/src/sys/unix/selector/kqueue.rs

Line 333 in c9831c7

pub type Event = libc::kevent;

Unless libc is going to implement Send for kevent we'll have to add a wrapper type or add the implementation to the public Event.

[Darksonn]

The type alias isn't exposed publicly. We should probably implement Send for the wrapper type in event/event.rs.

[Thomasdezeeuw]

We should probably implement Send for the wrapper type in event/event.rs.

I'm little hesitant to do that because if we ever add a new implementation we blindly overwrite Send and Sync, that's why I prefer to do it in the underlying implementation (in src/sys).

[Darksonn]

In that case it probably shouldn't be Send/Sync on any platform, but we can't really make that change.

[hulxv]

I see making a kevent wrapper is a good solution, as I see the udata is used for the token only as usize.

mio/src/sys/unix/selector/kqueue.rs

Lines 372 to 374 in c9831c7

	pub fn token(event: &Event) -> Token {
	Token(event.udata as usize)
	}

So instead of making Event an alias for libc::kevent, we can make a wrapper instead of that, and we can do something like that:

pub struct Event {
    pub ident: uintptr_t,
    pub filter: i16,
    pub flags: u16,
    pub fflags: u32,
    pub data: intptr_t,
    pub udata: usize,
}

impl From<libc::kevent> for Event {
    fn from(event: kevent) -> Self {
        Self {
            ident: event.ident,
            filter: event.filter,
            flags: event.flags,
            fflags: event.fflags,
            data: event.data,
            udata: event.udata as usize,
        }
    }
}

[Thomasdezeeuw]

Why do we need to define all the fields? We can do struct Kevent(libc::kevent), like we do with sys::Events.

[hulxv]

Why do we need to define all the fields? We can do struct Kevent(libc::kevent), like we do with sys::Events.

make sense. I did that in #1906