NetworkBehaviour event queue and polling

[b-zee]

When writing NetworkBehaviour, it's common to queue events and yield them when polled. The ping behaviour is a good example:

rust-libp2p/protocols/ping/src/lib.rs

Lines 81 to 82 in 0b44564

	/// Queue of events to yield to the swarm.
	events: VecDeque<Event>,

rust-libp2p/protocols/ping/src/lib.rs

Lines 151 to 157 in 0b44564

	fn poll(&mut self, _: &mut Context<'_>) -> Poll<ToSwarm<Self::ToSwarm, THandlerInEvent<Self>>> {
	if let Some(e) = self.events.pop_back() {
	Poll::Ready(ToSwarm::GenerateEvent(e))
	} else {
	Poll::Pending
	}
	}

In the case of ping, this queue is pushed onto from on_connection_handler_event.

---

Now what I'm pondering on is why this 'works'. There is no waker registered in poll, so in theory poll never gets called again ... right?

I have found some existing discussion on this here: #5147 (comment):

libp2p::Swarm guarantees that the behaviour is polled again after any changes to it via the internal APIs (i.e. all functions of the NetworkBehaviour trait).

However, I can't really verify/understand this pattern in Swarm::poll_next_event.

I assume technically the solution is to use a channel which the events are pushed on. If this channel is polled in poll then it will register a waker for whenever something is pushed in the channel. And so based on the above comment by Thomas, that's needed if events are queued outside of the NetworkBehaviour trait methods impls.

[elenaf9]

However, I can't really verify/understand this pattern in Swarm::poll_next_event.

It's because poll_next always continues from the top after any events in pool (e.g. new connection events) or events on established connections. And then all behaviors are polled again. Does that make sense?

I assume technically the solution is to use a channel which the events are pushed on. If this channel is polled in poll then it will register a waker for whenever something is pushed in the channel. And so based on the above comment by Thomas, that's needed if events are queued outside of the NetworkBehaviour trait methods impls.

Regarding the last part of queuing events outside of the NetworkBehavior trait method impls:
It kinda depends on your implementation. If the events are queued by methods that the user calls, then you might not need to to register a waker. The reason is that in order to call such a method, the user needs to stop polling the swarm (since poll_next requires a mut reference). After the method call, the user will continue polling the swarm from the top anyway, and thus also all behaviors.

Still, using a channel or simply storing the waker and waking it when a new event is queued is the safest variant.

[b-zee]

Thanks for the detailed explanation. I understand now. 🙏

The crux for me is that a waker is task-related (vs future specific) and all polling happening on a single thread. Waking might only be required when events come in from another thread/task.

Wakers being task-specific is something I just realized and it makes sense. The only exception I came across is FuturesUnordered. If I interpret the code correctly, it has wakers for all the futures in that object, so when one future is ready, it doesn't need to poll all features in the set.

(In theory Swarm could be optimized to use behaviour-specific wakers, so it doesn't need to poll all when one behaviour woke up the task. But that's all theory! 😉 )

[elenaf9]

(In theory Swarm could be optimized to use behaviour-specific wakers, so it doesn't need to poll all when one behaviour woke up the task. But that's all theory! 😉 )

Yes. There has been some discussion about this in #4990 🙂