tcp/udpsvd: robustify SIGCHLD handling

function                                             old     new   delta
if_verbose_print_connection_status                     -      40     +40
tcpudpsvd_main                                      1798    1794      -4
connection_status                                     31       -     -31
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 0/1 up/down: 40/-35)              Total: 5 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko 2021-06-05 15:24:04 +02:00
parent ac444861b0
commit d3e1090308
2 changed files with 27 additions and 11 deletions

View File

@ -216,17 +216,25 @@ enum {
OPT_K = (1 << 16), OPT_K = (1 << 16),
}; };
static void connection_status(void) static void if_verbose_print_connection_status(void)
{ {
/* "only 1 client max" desn't need this */ if (verbose) {
if (cmax > 1) /* "only 1 client max" desn't need this */
bb_error_msg("status %u/%u", cnum, cmax); if (cmax > 1)
bb_error_msg("status %u/%u", cnum, cmax);
}
} }
/* SIGCHLD handler is reentrancy-safe because SIGCHLD is unmasked
* only over accept() or recvfrom() calls, not over memory allocations
* or printouts. Do need to save/restore errno in order not to mangle
* these syscalls' error code, if any.
*/
static void sig_child_handler(int sig UNUSED_PARAM) static void sig_child_handler(int sig UNUSED_PARAM)
{ {
int wstat; int wstat;
pid_t pid; pid_t pid;
int sv_errno = errno;
while ((pid = wait_any_nohang(&wstat)) > 0) { while ((pid = wait_any_nohang(&wstat)) > 0) {
if (max_per_host) if (max_per_host)
@ -236,8 +244,8 @@ static void sig_child_handler(int sig UNUSED_PARAM)
if (verbose) if (verbose)
print_waitstat(pid, wstat); print_waitstat(pid, wstat);
} }
if (verbose) if_verbose_print_connection_status();
connection_status(); errno = sv_errno;
} }
int tcpudpsvd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int tcpudpsvd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
@ -458,7 +466,7 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
xconnect(0, &remote.u.sa, sa_len); xconnect(0, &remote.u.sa, sa_len);
/* hole? at this point we have no wildcard udp socket... /* hole? at this point we have no wildcard udp socket...
* can this cause clients to get "port unreachable" icmp? * can this cause clients to get "port unreachable" icmp?
* Yup, time window is very small, but it exists (is it?) */ * Yup, time window is very small, but it exists (does it?) */
/* ..."open new socket", continued */ /* ..."open new socket", continued */
xbind(sock, &lsa->u.sa, sa_len); xbind(sock, &lsa->u.sa, sa_len);
socket_want_pktinfo(sock); socket_want_pktinfo(sock);
@ -491,8 +499,7 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
if (pid != 0) { if (pid != 0) {
/* Parent */ /* Parent */
cnum++; cnum++;
if (verbose) if_verbose_print_connection_status();
connection_status();
if (hccp) if (hccp)
hccp->pid = pid; hccp->pid = pid;
/* clean up changes done by vforked child */ /* clean up changes done by vforked child */
@ -586,8 +593,14 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
xdup2(0, 1); xdup2(0, 1);
/* Restore signal handling for the to-be-execed process */
signal(SIGPIPE, SIG_DFL); /* this one was SIG_IGNed */ signal(SIGPIPE, SIG_DFL); /* this one was SIG_IGNed */
/* Non-ignored signals revert to SIG_DFL on exec anyway */ /* Non-ignored signals revert to SIG_DFL on exec anyway
* But we can get signals BEFORE execvp(), this is unlikely
* but it would invoke sig_child_handler(), which would
* check waitpid(WNOHANG), then print "status N/M" if verbose.
* I guess we can live with that possibility.
*/
/*signal(SIGCHLD, SIG_DFL);*/ /*signal(SIGCHLD, SIG_DFL);*/
sig_unblock(SIGCHLD); sig_unblock(SIGCHLD);

View File

@ -380,7 +380,10 @@ static void startservice(struct svdir *s)
xdup2(logpipe.wr, 1); xdup2(logpipe.wr, 1);
} }
} }
/* Non-ignored signals revert to SIG_DFL on exec anyway */ /* Non-ignored signals revert to SIG_DFL on exec anyway.
* But we can get signals BEFORE execl(), this is unlikely
* but wouldn't be good...
*/
/*bb_signals(0 /*bb_signals(0
+ (1 << SIGCHLD) + (1 << SIGCHLD)
+ (1 << SIGTERM) + (1 << SIGTERM)