mainloop: fix io_uring cleanup handling

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/mainloop.c b/src/lxc/mainloop.c
index 28f198c4ee73b864bc4777a5e34707b368ea321c..0fc47b7b64e96648dd4166b25357a9267a8934a1 100644 (file)
--- a/src/lxc/mainloop.c
+++ b/src/lxc/mainloop.c
@@ -37,11 +37,10 @@ struct mainloop_handler {
 static int __io_uring_disarm(struct lxc_async_descr *descr,
                             struct mainloop_handler *handler);
 
-static void delete_handler(struct lxc_async_descr *descr,
-                          struct mainloop_handler *handler, bool oneshot)
+static int disarm_handler(struct lxc_async_descr *descr,
+                         struct mainloop_handler *handler, bool oneshot)
 {
        int ret = 0;
-       struct lxc_list *list;
 
        if (descr->type == LXC_MAINLOOP_IO_URING) {
                /*
@@ -58,20 +57,39 @@ static void delete_handler(struct lxc_async_descr *descr,
                ret = epoll_ctl(descr->epfd, EPOLL_CTL_DEL, handler->fd, NULL);
        }
        if (ret < 0)
-               SYSWARN("Failed to delete \"%d\" for \"%s\"", handler->fd, handler->handler_name);
+               return syswarn_ret(-1, "Failed to disarm %d for \"%s\" handler",
+                                  handler->fd, handler->handler_name);
+
+       TRACE("Disarmed %d for \"%s\" handler", handler->fd, handler->handler_name);
+       return 0;
+}
+
+static void delete_handler(struct mainloop_handler *handler)
+{
+       struct lxc_list *list;
 
        if (handler->cleanup) {
+               int ret;
+
                ret = handler->cleanup(handler->fd, handler->data);
                if (ret < 0)
-                       SYSWARN("Failed to call cleanup \"%s\" handler", handler->handler_name);
+                       SYSWARN("Failed to cleanup %d for \"%s\" handler", handler->fd, handler->handler_name);
        }
 
+       TRACE("Deleted %d for \"%s\" handler", handler->fd, handler->handler_name);
        list = move_ptr(handler->list);
        lxc_list_del(list);
        free(list->elem);
        free(list);
 }
 
+static inline void cleanup_handler(struct lxc_async_descr *descr,
+                                  struct mainloop_handler *handler, bool oneshot)
+{
+       if (disarm_handler(descr, handler, oneshot) == 0)
+               delete_handler(handler);
+}
+
 #ifndef HAVE_LIBURING
 static inline int __lxc_mainloop_io_uring(struct lxc_async_descr *descr,
                                          int timeout_ms)
@@ -215,7 +233,7 @@ static int __lxc_mainloop_io_uring(struct lxc_async_descr *descr, int timeout_ms
 
        for (;;) {
                int ret;
-               __s32 mask = 0;
+               __s32 res = 0;
                bool oneshot = false;
                struct io_uring_cqe *cqe = NULL;
                struct mainloop_handler *handler = NULL;
@@ -236,53 +254,59 @@ static int __lxc_mainloop_io_uring(struct lxc_async_descr *descr, int timeout_ms
 
                ret     = LXC_MAINLOOP_CONTINUE;
                oneshot = !(cqe->flags & IORING_CQE_F_MORE);
-               mask    = cqe->res;
+               res     = cqe->res;
                handler = io_uring_cqe_get_data(cqe);
                io_uring_cqe_seen(descr->ring, cqe);
 
-               switch (mask) {
-               case -ECANCELED:
-                       handler->flags |= CANCEL_RECEIVED;
-                       TRACE("Canceled \"%s\" handler", handler->handler_name);
-                       goto out;
-               case -ENOENT:
-                       handler->flags = CANCEL_SUCCESS | CANCEL_RECEIVED;
-                       TRACE("No sqe for \"%s\" handler", handler->handler_name);
-                       goto out;
-               case -EALREADY:
-                       TRACE("Repeat sqe remove request for \"%s\" handler", handler->handler_name);
-                       goto out;
-               case 0:
-                       handler->flags |= CANCEL_SUCCESS;
-                       TRACE("Removed \"%s\" handler", handler->handler_name);
-                       goto out;
-               default:
-                       /*
-                        * We need to always remove the handler for a
-                        * successful oneshot request.
-                        */
-                       if (oneshot)
-                               handler->flags = CANCEL_SUCCESS | CANCEL_RECEIVED;
-               }
-
-               ret = handler->callback(handler->fd, mask, handler->data, descr);
-               switch (ret) {
-               case LXC_MAINLOOP_CONTINUE:
-                       /* We're operating in oneshot mode so we need to rearm. */
-                       if (oneshot && __io_uring_arm(descr, handler, true))
-                               return -1;
-                       break;
-               case LXC_MAINLOOP_DISARM:
-                       if (has_exact_flags(handler->flags, (CANCEL_SUCCESS | CANCEL_RECEIVED)))
-                               delete_handler(descr, handler, oneshot);
-                       break;
-               case LXC_MAINLOOP_CLOSE:
-                       return log_trace(0, "Closing from \"%s\"", handler->handler_name);
-               case LXC_MAINLOOP_ERROR:
-                       return syserror_ret(-1, "Closing with error from \"%s\"", handler->handler_name);
+               if (res <= 0) {
+                       switch (res) {
+                       case 0:
+                               TRACE("Removed \"%s\" handler", handler->handler_name);
+                               handler->flags |= CANCEL_SUCCESS;
+                               if (has_exact_flags(handler->flags, (CANCEL_SUCCESS | CANCEL_RECEIVED)))
+                                       delete_handler(handler);
+                               break;
+                       case -EALREADY:
+                               TRACE("Repeat sqe remove request for \"%s\" handler", handler->handler_name);
+                               break;
+                       case -ECANCELED:
+                               TRACE("Canceled \"%s\" handler", handler->handler_name);
+                               handler->flags |= CANCEL_RECEIVED;
+                               if (has_exact_flags(handler->flags, (CANCEL_SUCCESS | CANCEL_RECEIVED)))
+                                       delete_handler(handler);
+                               break;
+                       case -ENOENT:
+                               TRACE("No sqe for \"%s\" handler", handler->handler_name);
+                               break;
+                       default:
+                               WARN("Received unexpected return value %d in cqe for \"%s\" handler",
+                                    res, handler->handler_name);
+                               break;
+                       }
+               } else {
+                       ret = handler->callback(handler->fd, res, handler->data, descr);
+                       switch (ret) {
+                       case LXC_MAINLOOP_CONTINUE:
+                               /* We're operating in oneshot mode so we need to rearm. */
+                               if (oneshot && __io_uring_arm(descr, handler, true))
+                                       return -1;
+                               break;
+                       case LXC_MAINLOOP_DISARM:
+                               disarm_handler(descr, handler, oneshot);
+                               if (oneshot)
+                                       delete_handler(handler);
+                               break;
+                       case LXC_MAINLOOP_CLOSE:
+                               return log_trace(0, "Closing from \"%s\"", handler->handler_name);
+                       case LXC_MAINLOOP_ERROR:
+                               return syserror_ret(-1, "Closing with error from \"%s\"", handler->handler_name);
+                       default:
+                               WARN("Received unexpected return value %d from \"%s\" handler",
+                                    ret, handler->handler_name);
+                               break;
+                       }
                }
 
-       out:
                if (lxc_list_empty(&descr->handlers))
                        return error_ret(0, "Closing because there are no more handlers");
        }
@@ -314,7 +338,7 @@ static int __lxc_mainloop_epoll(struct lxc_async_descr *descr, int timeout_ms)
                                                handler->data, descr);
                        switch (ret) {
                        case LXC_MAINLOOP_DISARM:
-                               delete_handler(descr, handler, false);
+                               cleanup_handler(descr, handler, false);
                                __fallthrough;
                        case LXC_MAINLOOP_CONTINUE:
                                break;