Mdns `InterfaceState` never being called again after returning `Poll::Pending`

[stormshield-frb]

Summary

When updating to 0.53.0, we encountered a curious bug with Mdns.

After sending an incorrect Mdns packet, which is dropped (as expected), no packet is ever received by the peer after that, even if it is correct.

Expected behavior

An incorrect Mdns packet should not stop the polling of an InterfaceState.

Actual behavior

The polling of an InterfaceState is stopped after receiving an incorrect Mdns packet.

Relevant log output

No response

Possible Solution

Members of our team diagnosed the problem and managed to find the solution. It seems a dormant bug was present for a long time in the poll implementation of InterfaceState but it was only awakened with the merge of #4623.

rust-libp2p/protocols/mdns/src/behaviour/iface.rs

Lines 313 to 325 in cddc306

	Poll::Ready(Err(err)) if err.kind() == std::io::ErrorKind::WouldBlock => {
	// No more bytes available on the socket to read
	}
	Poll::Ready(Err(err)) => {
	tracing::error!("failed reading datagram: {}", err);
	}
	Poll::Ready(Ok(Err(err))) => {
	tracing::debug!("Parsing mdns packet failed: {:?}", err);
	}
	Poll::Ready(Ok(Ok(None))) | Poll::Pending => {}
	}
	return Poll::Pending;

As we can see in the above code sample, many Poll::Ready are handled and returned as Poll::Pending. However, in the rust documentation, we can read that "When a function returns Pending, the function must also ensure that the current task is scheduled to be awoken when progress can be made". When looking at the current implementation, we can safely say that, when a Poll::Ready is caught and converted into a Poll::Pending, no scheduling is done to ensure the task will be woken up.

Even if this bug has been present for a long time, it has not caused any problems up until now. Indeed, before the previously mentioned PR, all InterfaceState were stored in the iface_states HashMap and were polled no matter what their previous returned value was (since poll was called in a loop over each InterfaceState). So, even if the poll method of InterfaceState had not scheduled its own awakening, it would still be called at the next iteration of the poll method of the mdns Behaviour.

However, now that each InterfaceState is spawned in its own task, nobody is calling poll when it returns Poll::Pending. That is why returning a Poll::Pending when internally getting a Poll::Ready result in the poll method of InterfaceState never being called again, causing packets to never be handled.

Version

v0.53.0 (master)

Would you like to work on fixing this bug ?

Yes

[thomaseizinger]

Wow! Well done on spotting this. That is indeed a bug! :)