Skip to content

Commit

Permalink
folly: io_uring: only call readCallback if it did not change
Browse files Browse the repository at this point in the history
Summary:
Ucache with io_uring is crashing with this stack trace:

```
(lldb) bt
* thread #1, name = 'ucache', stop reason = signal SIGSEGV: address not mapped to object
  * frame #0: 0x0000000000000000
    frame #1: 0x00007f517ba44560 libc.so.6`__restore_rt at libc_sigaction.c:13
    frame #2: 0x000000000a16e249 ucache`folly::AsyncIoUringSocket::ReadSqe::callback(this=0x00007f498c5955e0, cqe=<unavailable>) at AsyncIoUringSocket.cpp:639
    frame #3: 0x000000000964196a ucache`folly::IoUringBackend::getActiveEvents(folly::IoUringBackend::WaitForEventsMode) [inlined] folly::IoSqeBase::internalCallback(this=<unavailable>, cqe=<unavailable>) at IoUringBackend.cpp:0
    frame #4: 0x000000000964195c ucache`folly::IoUringBackend::getActiveEvents(folly::IoUringBackend::WaitForEventsMode) [inlined] folly::IoUringBackend::internalProcessCqe(this=0x00007f4a8f989c00, maxGet=4294967295, mode=NORMAL) at IoUringBackend.cpp:1556
    frame #5: 0x000000000964190f ucache`folly::IoUringBackend::getActiveEvents(this=0x00007f4a8f989c00, waitForEvents=<unavailable>) at IoUringBackend.cpp:1527
    frame facebook#6: 0x0000000009640ac9 ucache`folly::IoUringBackend::eb_event_base_loop(this=0x00007f4a8f989c00, flags=1) at IoUringBackend.cpp:1178
    frame facebook#7: 0x000000001b61547c ucache`folly::EventBase::loopMain(this=0x00007f4a8f97af00, flags=0, options=<unavailable>) at EventBase.cpp:0
    frame facebook#8: 0x000000001bab9a2d ucache`folly::EventBase::loopBody(this=0x00007f4a8f97af00, flags=0, options=(ignoreKeepAlive = false, allowSuspension = false)) at EventBase.cpp:513
    frame facebook#9: 0x000000000a3a6c90 ucache`folly::EventBase::loop(this=<unavailable>) at EventBase.cpp:474
    frame facebook#10: 0x000000000a3a75f7 ucache`folly::EventBase::loopForever(this=<unavailable>) at EventBase.cpp:781
    frame facebook#11: 0x000000000a436907 ucache`folly::IOThreadPoolExecutor::threadRun(this=0x00007f4da3fce1c0, thread=<unavailable>) at IOThreadPoolExecutor.cpp:240
    frame facebook#12: 0x0000000009ed68b8 ucache`void std::__invoke_impl<void, void (folly::ThreadPoolExecutor::*&)(std::shared_ptr<folly::ThreadPoolExecutor::Thread>), folly::ThreadPoolExecutor*&, std::shared_ptr<folly::ThreadPoolExecutor::Thread>&>((null)=<unavailable>, __f=<unavailable>, __t=<unavailable>, __args=<unavailable>) at invoke.h:74
    frame facebook#13: 0x00007f517b6df4e5 libstdc++.so.6`std::execute_native_thread_routine(__p=0x00007f4da01bdbe0) at thread.cc:82:18
    frame facebook#14: 0x00007f517ba9abc9 libc.so.6`start_thread(arg=<unavailable>) at pthread_create.c:434:8
    frame facebook#15: 0x00007f517bb2cf5c libc.so.6`__clone3 at clone3.S:81
```

In frame 2 the offending function is `ReadSqe::callback()`: https://fburl.com/code/4fb46hdq

The problem is that `readCallback_` is null:

```
(lldb) fr v readCallback_
(folly::AsyncReader::ReadCallback *) readCallback_ = nullptr
```

However, `readCallback_` is checked earlier in the the function `ReadSqe::callback()`: https://fburl.com/code/dhnndk29

`AsyncIoUringSocket::readError()` now fails all pending writes and calls `writeErr()`, whose implementation can then close out a socket and call `AsyncIoUringSocket::setReadCB(nullptr)` to change the callback.

Move the call to `AsyncIoUringSocket::readError()` after the call to `readCallback_`.

Reviewed By: dmm-fb

Differential Revision: D60880377

fbshipit-source-id: 42d2f430cd3dc3f4787d0e4c29da7c8ef31ba9a8
  • Loading branch information
spikeh authored and facebook-github-bot committed Aug 12, 2024
1 parent 6d0a1a8 commit 7ac9203
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions folly/io/async/AsyncIoUringSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -639,9 +639,6 @@ void AsyncIoUringSocket::ReadSqe::callback(const io_uring_cqe* cqe) noexcept {
} else if (res < 0) {
// assume ECANCELED is not an unrecoverable error state, but we do still
// have to propogate to the callback as they presumably called the cancel.
if (parent_ && res != -ECANCELED) {
parent_->readError();
}
AsyncSocketException::AsyncSocketExceptionType err;
std::string error;
switch (res) {
Expand All @@ -660,6 +657,9 @@ void AsyncIoUringSocket::ReadSqe::callback(const io_uring_cqe* cqe) noexcept {
break;
}
readCallback_->readErr(AsyncSocketException(err, std::move(error)));
if (parent_ && res != -ECANCELED) {
parent_->readError();
}
} else {
uint64_t const cb_was = setReadCbCount_;
bytesReceived_ += res;
Expand Down

0 comments on commit 7ac9203

Please sign in to comment.