wl: do not close FDs that were hungup

Before this CL, the WlState object would close VFDs that had been hungup
on the remote end as a means to removing the underlying FD from the
PollContext. However, this has some unintended side-effects. For one,
the guest would later try to delete the VFD after it was closed, which
was a double-free. Another was that every pending message that was
waiting to enter the virtio queue would get dropped if it was destined
for the closed VFD. This was especially bad if the virtio queue became
full because data would get dropped when a VFD was hungup before the
guest had any chance to read it.

This CL leaves the hungup VFDs (and therefore their pending message) as
is, but removes it from the PollContext if there is nothing left to read.
No data is removed until after the guest explicitly closes the VFD.

TEST=paste 100k characters into a guest app from Chrome
BUG=chromium:849317

Change-Id: I20e3bc7c32c3f654f88f6ef9cdfcb853f2d52f09
Reviewed-on: https://chromium-review.googlesource.com/1088308
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
This commit is contained in:
Zach Reizner 2018-06-05 19:52:47 -07:00 committed by chrome-bot
parent 90c50419d4
commit 92068bca00

View file

@ -996,14 +996,11 @@ impl WlState {
for event in events.as_ref().iter_hungup() {
if !event.readable() {
let vfd_id = event.token();
if let Err(e) = self.close(vfd_id) {
warn!("failed to close vfd: {:?}", e)
if let Some(fd) = self.vfds.get(&vfd_id).and_then(|vfd| vfd.poll_fd()) {
if let Err(e) = self.poll_ctx.delete(fd) {
warn!("failed to remove hungup vfd from poll context: {:?}", e);
}
}
// Once the vfd has been hungup and there is nothing left to read, it's time to send
// the HUP event over virtio. It's very important that this in_queue entry gets
// pushed /after/ we self.close the vfd_id because part of the close operation for a
// vfd is removing all pending in_queue entries related to the vfd, regardless of
// type.
self.in_queue.push_back((vfd_id, WlRecv::Hup));
}
}