Skip to content

Sleep in runsv causes race-condition that can keep service persistently down #33

Description

@nickh2000

I have found a race condition that can occur when one calls sv up <service> and sv down <service> within 1 second while runsv is sleeping. This race-condition will lead to a service staying down even when the most recent call was sv up <service>. I have outlined the events which create this race-condition below:

  1. If a service unexpectedly terminates within <1 second of starting, runsv will sleep for 1 second to avoid starving the CPU:
    https://github.com/void-linux/runit/blob/2b8000f1ebd07fd68ee0e3c32737d97bcd1687fb/src/runsv.c#L553C1-L573C59
      child =wait_nohang(&wstat);
      if (!child) break;
      if ((child == -1) && (errno != error_intr)) break;
      if (child == svd[0].pid) {
        svd[0].pid =0;
        pidchanged =1;
        svd[0].wstat =wstat;
        svd[0].ctrl &=~C_TERM;
        if (svd[0].state != S_FINISH)
          if ((fd =open_read("finish")) != -1) {
            close(fd);
            svd[0].state =S_FINISH;
            update_status(&svd[0]);
            continue;
          }
        svd[0].state =S_DOWN;
        taia_uint(&deadline, 1);
        taia_add(&deadline, &svd[0].start, &deadline);
        taia_now(&svd[0].start);
        update_status(&svd[0]);
        **if (taia_less(&svd[0].start, &deadline)) sleep(1);**
  1. While runsv sleeps, let's suppose another process calls sv down <service>, which writes 'd' to 'supervise/control'.
  2. Then, while runsv is still sleeping, let's say a process makes another call to sv up <service>. Since runsv is sleeping, its s->want (svstatus[17]) has not been updated to S_DOWN. In sv.c, we see that svstatus[17] is still S_UP, so we do not write 'u' to 'supervise/control', and instead return early.
    https://github.com/void-linux/runit/blob/2b8000f1ebd07fd68ee0e3c32737d97bcd1687fb/src/sv.c#L251C1-L254C71
int control(char *a) {
  if (svstatus_get() <= 0) return(-1);
  if (svstatus[17] == *a)
    if (*a != 'd' || svstatus[18] == 1) return(0); /* once w/o term */
  1. Runsv wakes up and, since 'd' is the only character in 'supervise/control', runsv enters the S_DOWN state.

This is undesired behavior because the most recent call was sv up <service>. Since sv.c is not synchronous with the runsv state-machine, I do not think it should be relying on runsv's internal state in-order to write to the control pipe, else you lead to race-conditions such as this one.

I have created a pull request for my proposed change, which will allow 'up' to be written to 'supervise/control' even when svstatus[17] already shows 'up': #32

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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