axboe / liburing

Library providing helpers for the Linux kernel io_uring support
MIT License
2.86k stars 402 forks source link

io_uring_prep_cancel Not Returning -ECANCELED for Canceled Operations #1252

Closed Degoah closed 2 weeks ago

Degoah commented 3 weeks ago

Description:

When using io_uring_prep_cancel to cancel an ongoing recvmsg request in a separate thread, the completion queue event (cqe->res) does not always return the expected -ECANCELED code. Instead, it may return a different result, such as the completion result of the operation or another error code, even though the cancellation request was successfully submitted.

This behavior occurs in the following scenario:

  1. A recvmsg request is submitted to io_uring via io_uring_prep_recvmsg.
  2. A cancellation request is issued via io_uring_prep_cancel from a separate thread after a brief delay (e.g., 2 seconds).
  3. The completion event (cqe->res) does not return -ECANCELED, but another code such as a regular result or error, indicating that the request was not properly marked as canceled.

Code Snippet:

io_uring_sqe *sqe = io_uring_get_sqe(&ring);
io_uring_prep_recvmsg(sqe, sock, &msg, 0);
io_uring_submit(&ring);

// After some delay, in a separate thread:
io_uring_sqe *cancel_sqe = io_uring_get_sqe(&ring);
io_uring_prep_cancel(cancel_sqe, sqe, 0);
io_uring_submit(&ring);

//.....

int ret = io_uring_wait_cqe(&ring, &cqe);
if (ret < 0) {
 std::cerr << "Error waiting for completion event: " << strerror(-ret) << std::endl;
 break;
}

// Check if the request was canceled
if (cqe->res == -ECANCELED) {
    std::cout << "Request was successfully canceled" << std::endl;
} else if (cqe->res < 0) {
    std::cerr << "I/O operation failed: " << strerror(-cqe->res) << std::endl;
} else {
    std::cout << "I/O operation completed successfully with result: " << cqe->res << std::endl;
}

io_uring_cqe_seen(&ring, cqe);

Expected Behavior: When io_uring_wait_cqe receives the completion event for the canceled recvmsg operation, cqe->res should return -ECANCELED, indicating that the request was successfully canceled.

Actual Behavior: In some cases, cqe->res returns a different value, such as the result of the operation or another error code, even though a cancel request was submitted and expected to take effect.

This issue appears to cause confusion when trying to cancel operations, as the result does not correctly reflect that the operation was canceled. Any guidance on why this occurs or whether this is intended behavior would be helpful.

2nd Question/Issue What is the common best-practice approach to cancel ongoing io_uring_wait_cqe blocking-calls if the application needs to be shutdown cleanly.

Do we need to cancel first every single ongoing async SEQ-Request by invoking io_uring_prep_cancel and afterwards call io_uring_queue_exit. I cannot see any other way to to shutdown my application as the im blocked in my thread which blocks on io_uring_wait_cqe. Another thread just would issue io_uring_prep_cancel to make the blocking call exit.

Any suggestion are very appreciated.

Thx,

Rafael

axboe commented 3 weeks ago

Missing some key information here, like which kernel you are running. A few questions outside of that:

1) Are you sharing this ring between threads? If so, do you have proper synchronization in place? 2) Can you provide a trace of the above? Should be as simple as:

# echo 1 > /sys/kernel/debug/tracing/events/io_uring/enable
# run basic reproducer
# cat /sys/kernel/debug/tracing/trace > some_file

and then attach some_file in here.

Degoah commented 3 weeks ago

kernel: 6.1.20

  1. no!
  2. not able to do it as there is no such path /sys/kernel/debug/tracing/events/io_uring/enable
Degoah commented 3 weeks ago

To resolve this issue, maybe you can provide me an example. If you are aware that it works, so please provide to me a working example which acks that -ECANCELED gets returned as error code.:)

axboe commented 3 weeks ago

If you're not sharing the ring between threads, how is your cancelation request on another thread related to the first ring? One of these must be true:

1) You're sharing the ring and issuing a cancelation request on that same ring from another thread 2) The other thread is using another ring, and then how on earth would it be able to cancel a request on a different ring?

For an example, test/ in the liburing directory has many tests that include canceling a request. For a recvmsg example, test/recv-multishot.c has one.

Degoah commented 2 weeks ago

@axboe I will deliver a working example as soon as possible. Thx so far!

Degoah commented 2 weeks ago

If you're not sharing the ring between threads, how is your cancelation request on another thread related to the first ring? One of these must be true:

1. You're sharing the ring and issuing a cancelation request on that same ring from another thread

2. The other thread is using another ring, and then how on earth would it be able to cancel a request on a different ring?

For an example, test/ in the liburing directory has many tests that include canceling a request. For a recvmsg example, test/recv-multishot.c has one.

I just have a constallation where I start one thread. That thread is issuing a "io_uring_prep_recvmsg" request to the unique ring I manage in my app. Afterwards my application is blocking in "io_uring_wait_cqe". So fine so good. Next, I have also register a signal handler, which has access to the ring. In that signal handler I just do this here

        // ....
    // Cancel any pending io_uring requests related to the receiving of data.
    io_uring_sqe *sqe = io_uring_get_sqe(&m_ring_rx);
    if (sqe) {
        // If the producer thread is blocking in the io_uring_wait_cqe call,
        // we must unblock it by pushing a "cancel" request to the io_uring queue.
        io_uring_prep_cancel_fd(sqe, m_udp_socket, 0);
        // Submit the cancel request
        io_uring_submit(&m_ring_rx);
    }

Now, I would expect, that the blocking call to "io_uring_wait_cqe" returns, what it actually does.

                        //...
            // Wait for the completion event
            int ret = io_uring_wait_cqe(&m_ring_rx, &cqe);
            if (cqe->res == -ECANCELED) {
                std::cout << "rariru: do_receive(): ECANCELED " << std::endl;
            }
               //...    

Unfortunately, the "-ECANCELED" doesn't get set after the function "io_uring_wait_cqe" unblocks.

What Am I doing wrong here?

redbaron commented 2 weeks ago

I suspect return value is EINTR? then it might be expected

Degoah commented 2 weeks ago

Youre damn right. How could I overlook that;). So, for cancelling an ongoing blocking "io_uring_wait_cqe" there is actually no reason to check for "ECANCELLED", just for the EINTR instead. Is this the common way to do it?

redbaron commented 2 weeks ago

Common way to handle EINTR is to retry same operation

axboe commented 2 weeks ago

Additionally, always check the return value io_uring_wait_cqe() and related wait helpers for this very reason. You can't just do:

ret = io_uring_wait_cqe(ring, &cqe);
if (cqe->res)
    ....

as you don't know if CQE is valid unless ret == 0.

redbaron commented 2 weeks ago

just for my education, how safe it is to make io_uring calls from signal handler?

Degoah commented 2 weeks ago

Yes, it is unsafe. Looking for an alternative solution now, how to unblock the thread which is waiting in io_uring_wait_cqe() .

I have decided to use io_uring_wait_cqe() like this:

                // Wait for the completion event with a timeout
                int wait_ret = io_uring_wait_cqe_timeout(&m_ring, &cqe, &timeout);
                if (wait_ret == -ETIME) {
                    std::cout << "rariru: do_receive(): timeout" << std::endl;
                    // Timeout occurred, no completion event
                    continue;
                }

                if (wait_ret == -EINTR) {
                    // Timeout occurred, no completion event
                    std::cout << "rariru: do_receive(): EINTR" << std::endl;
                    finalize_io_buffer(buffer, true);
                    stop();

                    return;
                }

However, sending CTRL-C to the process, makes io_uring_wait_cqe_timeout call not unblock. Instead it is continuing waiting. So, the EINTR approach is not usable. Any ideas?

axboe commented 2 weeks ago

Getting a signal should very much unblock io_uring_wait_cqe_timeout(), it should abort waiting on any signal.

Degoah commented 2 weeks ago

...unfortunately it doesn't unblock. I tried it several times. Please give it a try and let me know what u got. Thx

axboe commented 2 weeks ago

Sure, can test. I'll try 6.1-stable, latest.

axboe commented 2 weeks ago
axboe@m2max-kvm ~> ./sig-wait
^C⏎
axboe@m2max-kvm ~ [SIGINT]> ./sig-wait
wait ret: -62
axboe@m2max-kvm ~> uname -r
6.1.112-06896-gaa4cd140bba5

Works for me, example I used below. This is using 6.1.112 as seen above.

Example:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <liburing.h>

int main(int argc, char *argv[])
{
    struct io_uring_sqe *sqe;
    struct io_uring_cqe *cqe;
    struct io_uring ring;
    struct __kernel_timespec ts;
    char buf[32];
    int fds[2];
    int ret;

    io_uring_queue_init(64, &ring, 0);

    if (pipe(fds) < 0) {
        perror("pipe");
        return 1;
    }

    sqe = io_uring_get_sqe(&ring);
    io_uring_prep_read(sqe, fds[0], buf, sizeof(buf), 0);
    io_uring_submit(&ring);

    ts.tv_sec = 5;
    ts.tv_nsec = 0;
    ret = io_uring_wait_cqe_timeout(&ring, &cqe, &ts);

    printf("wait ret: %d\n", ret);
    io_uring_queue_exit(&ring);
    return 0;
}
Degoah commented 2 weeks ago

I gave it a try, and it works as expected.

The issue was that I was using liburing and the io_uring_wait_cqe_timeout call in a multi-threaded setup. In this case, only the thread receiving the signal (with a dedicated signal handler registered) causes the interruption of any ongoing system calls. Since the thread that was blocking in io_uring_wait_cqe_timeout didn't receive the signal, it wasn't interrupted and didn't return EINTR.

Apologies for the confusion, and thanks so much for your effort! You're a great maintainer!

P.S.: This is my example application that wasn't working:

root@gl6:~# uname -r
6.1.20+g92eeaaec35c7+p0
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <liburing.h>
#include <signal.h>
#include <errno.h>
#include <pthread.h>

volatile sig_atomic_t got_signal = 0;

void handle_signal(int sig) {
    if (sig == SIGINT) {
        got_signal = 1;
    }
}

void* io_uring_thread_func(void* arg) {
    struct io_uring *ring = (struct io_uring *) arg;
    struct io_uring_cqe *cqe;
    int ret;

//  struct __kernel_timespec ts;
//  ts.tv_sec = 5;
//  ts.tv_nsec = 0;

    // Wait for completion event with a timeout
    ret = io_uring_wait_cqe(ring, &cqe);
    // ret = io_uring_wait_cqe_timeout(&ring, &cqe, &ts);

    if (got_signal) {
        printf("Received SIGINT in io_uring thread\n");
    }

    printf("wait ret: %d\n", ret);

    return NULL;
}

int main(int  , char * [])
{
    struct io_uring_sqe *sqe;
    struct io_uring ring;
    char buf[32];
    int fds[2];
    int ret;
    pthread_t io_uring_thread;

    // Set up the signal handler for SIGINT
    signal(SIGINT, handle_signal);

    io_uring_queue_init(64, &ring, 0);

    if (pipe(fds) < 0) {
        perror("pipe");
        return 1;
    }

    // Submit an sqe for reading from the pipe
    sqe = io_uring_get_sqe(&ring);
    io_uring_prep_read(sqe, fds[0], buf, sizeof(buf), 0);
    io_uring_submit(&ring);

    // Create a new thread to handle io_uring wait with timeout
    ret = pthread_create(&io_uring_thread, NULL, io_uring_thread_func, &ring);
    if (ret != 0) {
        perror("Failed to create thread");
        return 1;
    }

    // Wait for the io_uring thread to finish
    pthread_join(io_uring_thread, NULL);

    io_uring_queue_exit(&ring);
    return 0;
}
axboe commented 2 weeks ago

Thanks for the followup! Closing this one.