Skip to content
Snippets Groups Projects
Commit 91498882 authored by Robert Lord's avatar Robert Lord Committed by CQ bot account: commit-bot@chromium.org
Browse files

[fidl][rust] Fix deadlock in FIDL client

Previously, since a waker was reused between loops in recv_all, a
deadlock was possible:

    TASK 1  -> poll_recv_event
            <- pending, will wake task 1 on next chan.recv_from msg
    TASK 2  -> send MyRequest, poll_recv_msg_response
            <- pending, will wake task 2 on next chan.recv_from msg
    **event comes in, chan.recv_from's waker is called**
	    <- wake task 2
    TASK 2  -> poll_recv_msg_response
            <- pending, will wake task 2 on next chan.recv_from msg
	    <- but hey also event is ready so wake task 1
    TASK 1  -> poll_recv_event
            <- ready, will wake task 1 on next chan.recv_from msg
    **task 1 calls await!(Timer(1 year))**
    **MyRequest reply comes in, chan.recv_from's waker is called**
            <- wake task 1
	       ^ this will not poll or get recv_all called, since task 1
	         is waiting on a timer, not a FIDL event, so TASK 2 will
		 never be woken up with its now-ready response.

Eventually, after one year, task 1 will stop awaiting and either drop
the EventReceiver or await!() on it again, either of which will call
recv_all and wake task 2. This may be a contrived example, but I ran
into it when Task 1 requested a (futures-aware) lock that task 2 was
holding. Task 1 was blocked waiting on the lock, and task 2 never got
woken because of the logic above.

This CL makes several changes:

- recv_all now uses the same logic as wake_any to pick a Waker, instead
of using the initially-used waker, which may not be polling if the
Future is Ready after recv_all. In the example above, the last
poll_recv_event would cause the still-waiting task 2 to be registered as
the recv_from msg waker, instead of the no-longer-waiting task 1.
- poll_recv_msg_response and poll_recv_event now update their waker
*before* calling recv_all, so that recv_all can correctly select their
waker to be the next chan.recv_from waker if they'll still be Pending
after recv_all is called.
- When the EventListener is woken, it is now set to
EventListener::WillPoll to match the Received behavior in responses.
This means the comment in wake_any() is not necessary, since
already-called events that are now awaiting on something else will not
be selected in get_pending_worker(). EventListener::New was also renamed
to EventListener::WillPoll to clarify this new use of the state. This
also means the previous comment in wake_any is no longer relevant, since
an event receiver will only be the waker if got Pending on its last
poll.

Change-Id: Id3e23fd4575d220117207e43b6a5ce6b1e419f33
parent ac33ecdb
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment