Skip to content

Fix data loss on cancelled io#55

Merged
mstetsyuk merged 1 commit into
mainfrom
fix-lost-bytes-on-cancelled-io
May 30, 2026
Merged

Fix data loss on cancelled io#55
mstetsyuk merged 1 commit into
mainfrom
fix-lost-bytes-on-cancelled-io

Conversation

@mstetsyuk
Copy link
Copy Markdown
Member

Example of a potential data loss:

  1. We called FiberScheduler::read to read from the fd.
  2. Then called future->cancel().
  3. The cancel lands in the gap between the kernel writing the CQE to the ring and the scheduler processing the CQE.
  4. The cancel does future->result = nullptr.
  5. The scheduler does:
            int result = cqe->res;
            if (result >= 0)
            {
                if (future->result)
                {
                    *future->result = static_cast<uint64_t>(result);
                }
                future->set(0);
            }
  1. Effectively, we read data from the fd, but "dropped it on the floor", and subsequent reads won't read it.

It's enough to just remove future->result = nullptr from FiberScheduler::cancelIo. And it doesn't seem like that will break any invariant.

To add a deterministic red-green test, the simplest solution I found is to inject a hook on a future which runs after reading CQE from the ring and before processing it. This code is only compiled for test builds (i.e. debug or sanitizer). Might be potentially useful for other similar tests as well.

@mstetsyuk mstetsyuk changed the title Fix theoretical data loss on cancelled io Fix data loss on cancelled io May 29, 2026
@mstetsyuk mstetsyuk requested a review from vadimskipin May 29, 2026 14:02
Comment thread include/silk/fibers/fiber.h Outdated
Copy link
Copy Markdown
Collaborator

@vadimskipin vadimskipin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand this is a cancel/completion race. Yes, we must not clear result pointer and let kernel to resolve the outcome.

@vadimskipin
Copy link
Copy Markdown
Collaborator

I am not an experienced github user - can you squash commits before merging?

@mstetsyuk mstetsyuk force-pushed the fix-lost-bytes-on-cancelled-io branch 3 times, most recently from c47f008 to b6287fc Compare May 29, 2026 17:28
Comment thread src/fibers/tests/io-cancel-test.cpp
vadimskipin
vadimskipin previously approved these changes May 30, 2026
@mstetsyuk mstetsyuk merged commit 12bb257 into main May 30, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants