libxl: fd events: Suppress spurious fd events

Always recheck with poll() right before making the callback.

All sorts of things may have happened since poll() originally signaled
the fd.  We would like the main functional libxl code not to have to
worry about spurious wakeups.

In particular, this fixes a bug in the save/restore callout: the save
helper message reader operates with the fd in blocking mode.  In a
multithreaded program one thread might have eaten all the messages out
of the fd while another one is busy returning from poll and reacquiring
the libxl lock, possibly resulting in a deadlock.

(Also, we abolish the anomalous direct caller of efd->func.)

Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/libxl_event.c b/tools/libxl/libxl_event.c
index 2b2254eff60c1f59b5c1bec23490bff38cc90940..9ff4e78b7157077ab7b979d052b3fec75904f858 100644 (file)
--- a/tools/libxl/libxl_event.c
+++ b/tools/libxl/libxl_event.c
@@ -1121,12 +1121,15 @@ static int afterpoll_check_fd(libxl__poller *poller,
     return revents;
 }
 
-static void fd_occurs(libxl__egc *egc, libxl__ev_fd *efd, short revents)
+static void fd_occurs(libxl__egc *egc, libxl__ev_fd *efd, short revents_ign)
 {
-    DBG("ev_fd=%p occurs fd=%d events=%x revents=%x",
-        efd, efd->fd, efd->events, revents);
+    short revents_current = libxl__fd_poll_recheck(egc, efd->fd, efd->events);
 
-    efd->func(egc, efd, efd->fd, efd->events, revents);
+    DBG("ev_fd=%p occurs fd=%d events=%x revents_ign=%x revents_current=%x",
+        efd, efd->fd, efd->events, revents_ign, revents_current);
+
+    if (revents_current)
+        efd->func(egc, efd, efd->fd, efd->events, revents_current);
 }
 
 static void afterpoll_internal(libxl__egc *egc, libxl__poller *poller,
@@ -1255,10 +1258,7 @@ void libxl_osevent_occurred_fd(libxl_ctx *ctx, void *for_libxl,
     if (!ev) goto out;
     if (ev->fd != fd) goto out;
 
-    short current_revents = libxl__fd_poll_recheck(egc, ev->fd, ev->events);
-
-    if (current_revents)
-        ev->func(egc, ev, fd, ev->events, current_revents);
+    fd_occurs(egc, ev, revents_ign);
 
  out:
     CTX_UNLOCK;

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2986e3bbe12eaef24b01096db14c7d11e9d6b6e7..8eb38aae7b345b510051ac22101720f1919b3f88 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -177,6 +177,9 @@ typedef void libxl__ev_fd_callback(libxl__egc *egc, libxl__ev_fd *ev,
    *
    * It is not permitted to listen for the same or overlapping events
    * on the same fd using multiple different libxl__ev_fd's.
+   *
+   * (Spurious wakeups, and spurious bits set in revents, are
+   * suppressed by the libxl event core.)
    */
 struct libxl__ev_fd {
     /* caller should include this in their own struct */