Skip to content

cancelIo fix - cancellation sqe should be sent to the same ring as the original sqe#59

Merged
vadimskipin merged 1 commit into
ClickHouse:mainfrom
qkrorlqr:users/qkrorlqr/cancelIo-fix
Jun 1, 2026
Merged

cancelIo fix - cancellation sqe should be sent to the same ring as the original sqe#59
vadimskipin merged 1 commit into
ClickHouse:mainfrom
qkrorlqr:users/qkrorlqr/cancelIo-fix

Conversation

@qkrorlqr
Copy link
Copy Markdown
Contributor

@qkrorlqr qkrorlqr commented Jun 1, 2026

Otherwise cancellation returns ENOENT

ut added

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 1, 2026

CLA assistant check
All committers have signed the CLA.

Comment thread include/silk/fibers/fiber.h Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes cross-fiber I/O cancellation with io_uring by ensuring the cancel SQE is submitted to the same per-CPU ring that originally holds the operation’s SQE, preventing -ENOENT cancels that can leave waiters blocked indefinitely.

Changes:

  • Track which processor/ring an IoFuture was submitted to (IoFuture::submitProcessor).
  • Route cancelIo() submissions to the recorded ring and, when cancelling remotely, force-submit and wake the target processor thread.
  • Add a regression test that cancels a POLL_ADD from a different fiber (likely on a different CPU due to work-stealing).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/fibers/fiber.cpp Records SQE submit ring and routes cancel SQEs to the correct per-CPU io_uring ring.
include/silk/fibers/fiber.h Extends IoFuture with submitProcessor to support correct cancel routing.
src/fibers/tests/fiber-test.cpp Adds a cross-fiber cancellation regression test for poll()/cancel().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/fibers/tests/fiber-test.cpp Outdated
Comment thread src/fibers/tests/fiber-test.cpp Outdated
Comment thread src/fibers/fiber.cpp Outdated
Comment thread src/fibers/fiber.cpp Outdated
Comment thread src/fibers/fiber.cpp Outdated
@qkrorlqr qkrorlqr force-pushed the users/qkrorlqr/cancelIo-fix branch 6 times, most recently from 2b7868d to f791d77 Compare June 1, 2026 15:03
vadimskipin
vadimskipin previously approved these changes Jun 1, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

Comment thread include/silk/fibers/fiber.h
Comment thread src/fibers/fiber.cpp
Comment thread src/fibers/fiber.cpp
Comment thread src/fibers/tests/fiber-test.cpp Outdated
@vadimskipin vadimskipin merged commit e992c89 into ClickHouse:main Jun 1, 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.

4 participants