Skip to content

Commit fa70b79

Browse files
committed
Protect forking services from false crash detection during reload
The service_reload() sets svc_starting() to flag that a reload is in progress. The remaining changes use that flag to prevent the state machine from interpreting the old PID's death as a crash: service_monitor() doesn't zero the PID, service_step() RUNNING stays put instead of restarting, and PAUSED doesn't drop to WAITING for forking services. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
1 parent bd91395 commit fa70b79

1 file changed

Lines changed: 21 additions & 6 deletions

File tree

src/service.c

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,10 +1299,17 @@ static int service_reload(svc_t *svc)
12991299
}
13001300

13011301
if (!rc) {
1302-
/* Declare we're waiting for svc to re-assert/touch its pidfile */
1302+
/*
1303+
* Flag that we expect the service to (re-)assert its PID
1304+
* file. For forking services whose reload script kills and
1305+
* restarts the daemon, this prevents service_step() from
1306+
* treating the collected old PID as a crash.
1307+
*
1308+
* For services with managed pidfiles, also touch the file
1309+
* to trigger an inotify event that reasserts the condition.
1310+
*/
13031311
svc_starting(svc);
13041312

1305-
/* Service does not maintain a PID file on its own */
13061313
if (svc_has_pidfile(svc)) {
13071314
sched_yield();
13081315
touch(pid_file(svc));
@@ -2338,10 +2345,8 @@ void service_monitor(pid_t lost, int status)
23382345
/* Forking sysv/services declare themselves with pid:!/path/to/pid.file */
23392346
if (svc_is_forking(svc)) {
23402347
/* Likely start script exiting */
2341-
if (svc_is_starting(svc)) {
2342-
svc->pid = 0; /* Expect no more activity from this one */
2348+
if (svc_is_starting(svc))
23432349
goto cont;
2344-
}
23452350

23462351
logit(LOG_CONSOLE | LOG_NOTICE, "Stopped %s[%d]", svc_ident(svc, NULL, 0), lost);
23472352
}
@@ -3036,6 +3041,16 @@ int service_step(svc_t *svc)
30363041
}
30373042

30383043
if (!svc->pid) {
3044+
/*
3045+
* A reload script (e.g. frrinit.sh reload) may kill
3046+
* the old daemon and start a new one. When we collect
3047+
* the old PID, stay RUNNING and wait for the new PID
3048+
* file to appear -- pidfile_update_conds() will call
3049+
* service_forked() to update svc->pid.
3050+
*/
3051+
if (svc_is_starting(svc) && svc_is_forking(svc))
3052+
goto done;
3053+
30393054
if (svc_is_daemon(svc) || svc_is_tty(svc)) {
30403055
int rc = WEXITSTATUS(svc->status);
30413056

@@ -3124,7 +3139,7 @@ int service_step(svc_t *svc)
31243139
break;
31253140
}
31263141

3127-
if (!svc->pid) {
3142+
if (!svc->pid && !svc_is_forking(svc)) {
31283143
(*restart_cnt)++;
31293144
svc_set_state(svc, SVC_WAITING_STATE);
31303145
break;

0 commit comments

Comments
 (0)