Potential race condition of event registration and adding waker?

[npuichigo]

@cfsamson Would it be possible that after the interest is registered, the reactor quickly get the event and tries to call wake, but waker has not been added since lock here. Now that epoll is edge-level here, the event will never be received again so never wake the executor. I'm not sure if I'm right.

Asynchronous-Programming-in-Rust/ch10/a-rust-futures-bonus/src/http.rs

Lines 69 to 70 in 04d893c

	runtime::reactor().register(stream, Interest::READABLE, id);
	runtime::reactor().set_waker(waker, self.id);

[cfsamson]

Hi!

Good catch! As far as I can see you're right in theory. Even though it's unlikely in this example since a network call is so slow (even locally) that it's highly unlikely that you could get in trouble here. However, if you the response could return much quicker, you could have the situation that the reactor loop recieves an event, takes the lock on waker first and finds no wakers for that event and just goes back to sleep. In that case, we could risk never getting a notification again just as you reasoned.

As I see it, there are two options here:

1. Implement something like register_with_waker that takes the lock on wakers before registering interest and releases it when the waker is registered so that if the reactor recieves an event, it will block on the mutex until the waker is registered and it can take the lock.
2. Move the registration of interest and waker to before we make the call to write_request(), which is less robust and more of a hack.

        if self.stream.is_none() {
            println!("FIRST POLL - START OPERATION");
            // CHANGED
            let stream = (&mut self).stream.as_mut().unwrap();
            runtime::reactor().register(stream, Interest::READABLE, id);
            runtime::reactor().set_waker(waker, self.id);
            // ============
            self.write_request();
        }

The first is the best since it makes it less likely to make the same error again.

I'll note this down as an improvement for a second revision if it comes to that, for now, let's just leave this issue open so it's possible to find it.

Thanks again for reporting!