Skip to content

libev builds are fork-unsafe: processes share the default loop's backend, wedging reloads #491

Description

@citrus-it

I've been digging into a problem that I've seen running nsd on illumos systems
where I find it apparently wedged from time to time, seemingly related to a
reload event.

TL;TR

When NSD is built against libev, nsd_child_event_base() returns
ev_default_loop(EVFLAG_AUTO) rather than a fresh event base.
libev caches the default loop in a static, NSD never calls
event_reinit()/ev_loop_fork() after fork, and libev's libevent-compat
event_base_free() refuses to destroy the default loop.
Once any process has run one reload, every process it subsequently forks
inherits the same initialised loop and its open backend fd.

On illumos the backend is an event port, a kernel object shared across fork
with one-shot associations, so parent, serve children and later reloads all
port_getn() on a single shared port and consume each other's events.

The result on my secondaries was a permanently wedged reload handover,
duplicate main processes, unreaped zombie serve children, and serve
children that progressively stop answering queries. xfrd s) stays healthy,
so nsd-control keeps working and monitoring sees a live daemon.

The same sharing applies in principle to epoll fds for Linux+libev builds.
From what I can see, distributions generally link libevent, which is probably
why this hasn't been reported.

Description

I'm running NSD 4.14.2 (OmniOS packaged) built against libev 4.33 on
OmniOS stable r151058 serving around 75 zones.

When I see the hang, the nsd process tree looks like this:

5751  /opt/ooce/sbin/nsd            # xfrd (healthy)
5758  /opt/ooce/sbin/nsd            # stuck in server_reload quit-sync
  5762  <defunct>                   # old serve children, never reaped
  5763  <defunct>
  5761  /opt/ooce/sbin/nsd          # orphaned old-main (reload #2)
  8996  /opt/ooce/sbin/nsd          # old-main (reload #3), waiting on xfrd ack
  8997  /opt/ooce/sbin/nsd          # serve children
  8998  /opt/ooce/sbin/nsd

The stacks of these (abridged) are:

5758: portfs/port_getn(18) <- port_poll <- ev_run <- event_base_loop
        <- server_main+0x11c6
8997: portfs/port_getn(18) <- port_poll <- ev_run <- event_base_loop
        <- server_child <- restart_child_servers <- server_main+0x107e
8998: (same as 8997)
5761: pollsys <- ppoll <- netio_dispatch <- server_main+0x5de
8996: (same as 5761)
5751: portfs/port_getn(19) <- ... <- xfrd_init

5758, 8997 and 8998 are all inside libev and /all/ polling on fd 18.

If I look at the file descriptor tables for these with the kernel
debugger, I can see that they're all the same event port.

PID    FD  TYPE  VNODE            INFO
5758   18  PORT  fffffe2e5d44fe80 [event port (port=fffffe2e5d8ce740)]
8998   18  PORT  fffffe2e5d44fe80 [event port (port=fffffe2e5d8ce740)]
8997   18  PORT  fffffe2e5d44fe80 [event port (port=fffffe2e5d8ce740)]
8996   18  PORT  fffffe2e5d44fe80 [event port (port=fffffe2e5d8ce740)]
5761   18  PORT  fffffe2e5d44fe80 [event port (port=fffffe2e5d8ce740)]
5751   18  FIFO  fffffe2df6778140                 # xfrd

Reproduction

Reproduction is pretty easy on these systems. With a running nsd built
against libev it is only necessary to trigger two reloads in successfion in the
same server-main process. The first reload creates the default loop and the
serve children and quit-sync of the second reload share its backend. The
failure is timing-dependent but with illumos event ports it wedged for us on
the first or second attempt every time.

Suggested fix

Rather than handing out the default loop, create a loop per call. This will
also make the existing event_base_free() cleanup calls work.
Historically signal watchers had to be in the default loop but that constraint
was dropped in libev 4.x, and NSD only uses one loop per process.

--- a/server.c
+++ b/server.c
@@ -3287,7 +3287,7 @@
 #  else
        m = "?";
 #  endif
-#  ifdef MEMCLEAN
+#  if defined(MEMCLEAN) || defined(HAVE_EV_LOOP) || defined(HAVE_EV_DEFAULT_LOOP)
        event_base_free(b);
 #  endif
        return m;
@@ -3304,8 +3305,12 @@
        base = event_init(&secs, &now);
 #else
 #  if defined(HAVE_EV_LOOP) || defined(HAVE_EV_DEFAULT_LOOP)
-       /* libev */
-       base = (struct event_base *)ev_default_loop(EVFLAG_AUTO);
+       /* libev. Create a new loop rather than handing out the cached
+        * default loop. The default loop survives fork() with its backend
+        * still open and shared with the parent, and libev's event_base_free()
+        * refuses to destroy it, so every forked process would multiplex one
+        * kernel event object and steal events from its relatives. */
+       base = (struct event_base *)ev_loop_new(EVFLAG_AUTO);
 #  else
        /* libevent */
 #    ifdef HAVE_EVENT_BASE_NEW

I am also going to switch the OmniOS packages to use libevent as it seems
generally preferred.

Happy to open a PR with my proposed fix if you'd like.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions