impl Send for KEvent

[asomers]

carllerche/mio needs KEvent to be Send-able. It's safe to send because udata is always treated as a uintptr_t. The only ways to use udata that wouldn't be Send-able would also be unsafe because they would require casting udata to a pointer. So I think it's safe to add the Send trait to KEvent.

[asomers]

Both test failures were due to setup issues with rust 1.2.0, and were not related to my change.

[posborne]

@asomers Can you rebase on master? The CI issue should be fixed.

[posborne]

Actually, I think the build kicked off by homu should pass as it will create the merge commit and test that. Change looks good to me. Since KEvent has already changed with this release, I don't think additional release notes are required.

@homu r+

[homu]

📌 Commit f9b178b has been approved by posborne

[fiveop]

How is uintptr_t safe to send? It is a pointer.

[homu]

⌛ Testing commit f9b178b with merge 62a0168...

[asomers]

@fiveop uintptr_t is defined as an integer that's the same size as a pointer. So it's safe to send. The unsafe part is if you cast it back to a pointer type and dereference it.
http://en.cppreference.com/w/c/types/integer

[posborne]

@homu r- for now

[homu]

☀️ Test successful - status

[posborne]

@homu r-

[asomers]

@posborne what would you to be satisfied with this PR? I admit that I'm a Rust novice, but I'm pretty sure that uintptr_t is safe to send, since it's just an integer. If any user tries to store a pointer in udata, they'll still need an unsafe{} block when converting udata back into a pointer.

[posborne]

Hi @asomers I held off on the merge as there was more discussion that seems like it needed to take place and haven't gotten around to thinking about this one more for a few days (was hanging out with @kamalmarhubi and others at the Rust Belt Rust conference).

Looking at things again, I'm not sure the basis for your argument that KEvent is just a uintptr_t is coming from. KEvent is a newtype (https://aturon.github.io/features/types/newtype.html) wrapper around ::libc::kevent which is a structure with several members and, as such, is not safe to send itself. A reference/pointer to a KEvent might be safe to mark Send but that is not what your code indicates. The ident member of ::libc::kevent does boil down to a uintptr_t and, I think, should be safe to send already as it should be copyable.

Perhaps some more context on what you are trying to accomplish in mio could be useful. Is there a tracking issue for what you are trying to accomplish?

[asomers]

@posborne I don't understand your comment. You seem to be saying that all Structs with multiple elements are by default not Sendable, but they are. For example, the following code works:

use std::thread;

struct X {
  a: u32,
  b: u32
}

fn main() {
  let mut x = X{a: 0, b:0};
  thread::spawn(move || {
    x.a += 1;
  });
}

My problem is in carllerche/mio in test/test_notify.rs. The test_notify_capacity function creates an EventLoop, which contains a Vector ofKEvents. It fails to compile with an error that says std::marker::Send is not satisfied because of libc::kevent's *mut ::c_void member. Changing that one member to a uintptr_t fixes the compile error, so it has nothing to do with libc::kevent being a compound object.

test/test_notify.rs:97:18: 97:31 error: the trait bound *mut libc::c_void: std::marker::Send is not satisfied [E0277]
test/test_notify.rs:97 let handle = thread::spawn(move || {
^~~~~~~~~~~~~
test/test_notify.rs:97:18: 97:31 help: run rustc --explain E0277 to see a detailed explanation
test/test_notify.rs:97:18: 97:31 note: *mut libc::c_void cannot be sent between threads safely
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type libc::unix::bsd::freebsdlike::kevent
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type nix::sys::event::KEvent
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type alloc::raw_vec::RawVec<nix::sys::event::KEvent>
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type std::vec::Vec<nix::sys::event::KEvent>
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type mio::sys::unix::kqueue::Events
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type mio::Events
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type mio::deprecated::EventLoop<test_notify::test_notify_capacity::Capacity>
test/test_notify.rs:97:18: 97:31 note: required because it appears within the type [closure@test/test_notify.rs:97:32: 100:6 rx:std::sync::mpsc::Receiver<i32>, event_loop:mio::deprecated::EventLoop<test_notify::test_notify_capacity::Capacity>]
test/test_notify.rs:97:18: 97:31 note: required by std::thread::spawn
test/test_notify.rs:157:18: 157:31 error: the trait bound *mut libc::c_void: std::marker::Send is not satisfied [E0277]
test/test_notify.rs:157 let handle = thread::spawn(move || {
^~~~~~~~~~~~~
test/test_notify.rs:157:18: 157:31 help: run rustc --explain E0277 to see a detailed explanation
test/test_notify.rs:157:18: 157:31 note: *mut libc::c_void cannot be sent between threads safely
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type libc::unix::bsd::freebsdlike::kevent
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type nix::sys::event::KEvent
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type alloc::raw_vec::RawVec<nix::sys::event::KEvent>
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type std::vec::Vec<nix::sys::event::KEvent>
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type mio::sys::unix::kqueue::Events
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type mio::Events
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type mio::deprecated::EventLoop<test_notify::test_notify_drop::DummyHandler>
test/test_notify.rs:157:18: 157:31 note: required because it appears within the type [closure@test/test_notify.rs:157:32: 167:6 event_loop:mio::deprecated::EventLoop<test_notify::test_notify_drop::DummyHandler>, tx_exit_loop:std::sync::mpsc::Sender<()>, rx_drop_loop:std::sync::mpsc::Receiver<()>]
test/test_notify.rs:157:18: 157:31 note: required by std::thread::spawn
error: aborting due to 2 previous errors
Build failed, waiting for other jobs to finish...
error: Could not compile mio.

[posborne]

Ok, I see now. Pardon my ignorance, it's been a little while since I have had to grapple with Send semantics. Since we know that particular pointer is not aliased outside of the Nix KEvent object, we also know that guarantees provided by Rust to avoid data races apply for KEvent (the compiler could not know this on its own, so we need to tell it that).

Ok, I approve of the change and we can merge. Could you quickly add a line to the changelog saying that we know mark KEvent as Sendable? Also, can you update the comment to be C++-style (//) for consistency with the rest of the code. Nit picky but it all adds up.

Thanks for explanation and the change!

[posborne]

Thanks, to deal with the conflict, I went ahead and rebased your branch on upstream master. #453 should be merged shortly with your changes. Thanks!