Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix re-registration of internal events after fork #1599

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ceggers-arri
Copy link
Contributor

If adding an (internal) descriptor to epoll fails, mark it for not being re-registered after fork (this would throw an exception).

In my case, during notify_fork() the io_uring service (re)registers with the epoll reactor. As the eventfd descriptor is already registered, EPOLL_CTL_ADD fails with EEXIST (which is ignored here). During re-registration after fork, epoll (again) refuses the 2nd registration of the epoll descriptor and the child crashes because of an exception:

Call stack during fork_prepare:

void io_uring_service::notify_fork(
    asio::execution_context::fork_event fork_ev)
{
  switch (fork_ev)
  {
  case asio::execution_context::fork_prepare:
    {
...
      // Restart and eventfd operation.
      register_with_reactor();   // <-- trouble starts here...
    }
    break;

...
}
void io_uring_service::register_with_reactor()
{
#if !defined(ASIO_HAS_IO_URING_AS_DEFAULT)
  // ---> Return code is ignored here <---
  reactor_.register_internal_descriptor(reactor::read_op,
      event_fd_, reactor_data_, new event_fd_read_op(this));
#endif // !defined(ASIO_HAS_IO_URING_AS_DEFAULT)
}
int epoll_reactor::register_internal_descriptor(
    int op_type, socket_type descriptor,
    epoll_reactor::per_descriptor_data& descriptor_data, reactor_op* op)
{
  ...

  epoll_event ev = { 0, { 0 } };
  ev.events = EPOLLIN | EPOLLERR | EPOLLHUP | EPOLLPRI | EPOLLET;
  descriptor_data->registered_events_ = ev.events;
  ev.data.ptr = descriptor_data;
  int result = epoll_ctl(epoll_fd_, EPOLL_CTL_ADD, descriptor, &ev);
  if (result != 0)
    return errno;   // <--- descriptor_data doesn't reflect that epoll registration failed

  return 0;
}

Call stack during fork_child:

void epoll_reactor::notify_fork(
    asio::execution_context::fork_event fork_ev)
{
  if (fork_ev == asio::execution_context::fork_child)
  {
...
    // Re-register all descriptors with epoll.
    mutex::scoped_lock descriptors_lock(registered_descriptors_mutex_);
    for (descriptor_state* state = registered_descriptors_.first();
        state != 0; state = state->next_)
    {
      if (state->registered_events_ != 0)
      {
        ev.events = state->registered_events_;
        ev.data.ptr = state;
        int result = epoll_ctl(epoll_fd_,
            EPOLL_CTL_ADD, state->descriptor_, &ev);
        if (result != 0)
        {
          asio::error_code ec(errno,
              asio::error::get_system_category());
          asio::detail::throw_error(ec, "epoll re-registration");    // <--- bang!
        }
      }
    }
  }
}

Reproducer (should dump the output of the top command to a file):

#include <boost/asio.hpp>
#include <boost/process/v2.hpp>

int main(void)
{
	boost::system::error_code ec;
	boost::asio::io_context ioContext;
	boost::asio::stream_file outputFile(ioContext, "top.txt",
		boost::asio::stream_file::create | boost::asio::stream_file::write_only);

	boost::process::v2::process proc(ioContext,
		"/usr/bin/top", {"-H", "-b", "-n", "1"},
		boost::process::v2::process_stdio{{/*stdin default*/}, outputFile, {/*stderr default*/}});
	proc.wait();
}

If adding an (internal) descriptor to epoll fails, mark it for not being
re-registered after fork (this would throw an exception).

In my case, during notify_fork() the io_uring service (re)registers with
the epoll reactor.  As the eventfd descriptor is already registered,
EPOLL_CTL_ADD fails with EEXIST (which is ignored here).  During
re-registration after fork, epoll (again) refuses the 2nd registration
of the epoll descriptor and the child crashes because of an exception.
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.

1 participant