Skip to content

Commit

Permalink
Fix #4666 by draining pipe prior to writing to it
Browse files Browse the repository at this point in the history
  • Loading branch information
RedBeard0531 committed May 26, 2021
1 parent bfed18e commit fe96b09
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 21 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* Fixed an incorrect detection of multiple incoming links in a migration when changing a table to embedded and removing a link to it at the same time. ([#4694](https://github.com/realm/realm-core/issues/4694) since 10.0.0-beta.2)
* Fixed build failure with gcc-11
* Made Linux implementation of ExternalCommitHelper work with new versions of Linux that [changed epoll behavior](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6a965666b7e7475c2f8c8e724703db58b8a8a445), including Android 12 ([#4666](https://github.com/realm/realm-core/issues/4666))

### Breaking changes
* None.
Expand Down
55 changes: 34 additions & 21 deletions src/realm/object-store/impl/epoll/external_commit_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,30 +49,45 @@ using namespace realm::_impl;
} while (0)

namespace {
// Write a byte to a pipe to notify anyone waiting for data on the pipe

// Make writing to the pipe return -1 when there is no data to read, or no space in the buffer to write to, rather
// than blocking.
void make_non_blocking(int fd) {
int ret = fcntl(fd, F_SETFL, O_NONBLOCK);
if (ret == -1) {
throw std::system_error(errno, std::system_category());
}
}

// Write a byte to a pipe to notify anyone waiting for data on the pipe.
// But first consume all bytes in the pipe, since linux may only notify on transition from not ready to ready.
void notify_fd(int fd)
{
while (true) {
while (true) {
uint8_t buff[1024];
ssize_t actual = read(fd, buff, sizeof(buff));
if (actual == 0) {
break; // Not sure why we would see EOF here, but defer error handling to the writer.
}
if (actual < 0) {
int err = errno;
if (err == EWOULDBLOCK || err == EAGAIN)
break;
throw std::system_error(err, std::system_category());
}
}

char c = 0;
ssize_t ret = write(fd, &c, 1);
if (ret == 1) {
break;
}

// If the pipe's buffer is full, we need to read some of the old data in
// it to make space. We don't just read in the code waiting for
// notifications so that we can notify multiple waiters with a single
// write.
if (ret != 0) {
int err = errno;
if (err != EAGAIN) {
throw std::system_error(err, std::system_category());
}
}
std::vector<uint8_t> buff(1024);
auto actual = read(fd, buff.data(), buff.size());
if (actual == 0) {
throw std::runtime_error("Could not read from pipe");
REALM_ASSERT_RELEASE(ret < 0);
int err = errno;
if (err != EWOULDBLOCK && err != EAGAIN) {
throw std::system_error(err, std::system_category());
}
}
}
Expand Down Expand Up @@ -156,12 +171,7 @@ ExternalCommitHelper::ExternalCommitHelper(RealmCoordinator& parent)
throw std::system_error(errno, std::system_category());
}

// Make writing to the pipe return -1 when the pipe's buffer is full
// rather than blocking until there's space available
int ret = fcntl(m_notify_fd, F_SETFL, O_NONBLOCK);
if (ret == -1) {
throw std::system_error(errno, std::system_category());
}
make_non_blocking(m_notify_fd);

// Lock is inside add_commit_helper.
DaemonThread::shared().add_commit_helper(this);
Expand Down Expand Up @@ -189,6 +199,9 @@ ExternalCommitHelper::DaemonThread::DaemonThread()
m_shutdown_read_fd = pipe_fd[0];
m_shutdown_write_fd = pipe_fd[1];

make_non_blocking(m_shutdown_read_fd);
make_non_blocking(m_shutdown_write_fd);

epoll_event event{};
event.events = EPOLLIN;
event.data.fd = m_shutdown_read_fd;
Expand Down

0 comments on commit fe96b09

Please sign in to comment.