From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no autolearn=ham autolearn_force=no version=3.4.6 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id B90F61F69D for ; Tue, 17 Oct 2023 23:38:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org; s=selector1; t=1697585897; bh=OmCs2dO96JRjFjwftbh8aa1MUoL4rol+spYe+KQmUzQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=w4UjYaYvww67y6mRtS2Q3n3tp/ydax8hEyWX86Ku3fcARyWAlBVyrThr8TM7gcPEm xbX2i2AM1rb3QvRywY3ZmrPC0/Oy1YXc8xT9jyTjbwDX2W7swDcH0QMT7/2d26f//5 zWiTs4BowcAdpWAFLYoKaQJ4tg0YwZ7Bxn/Ht2vY= From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Date: Tue, 17 Oct 2023 23:37:55 +0000 Message-ID: <20231017233815.1637932-11-e@80x24.org> In-Reply-To: <20231017233815.1637932-1-e@80x24.org> References: <20231017233815.1637932-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: We can just close the socket FD to ensure things fail ASAP when a SIGTERM hits instead of wasting time making getppid(2) syscalls. --- lib/PublicInbox/XapHelper.pm | 7 +++---- lib/PublicInbox/xap_helper.h | 28 ++++++++++++++++------------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm index fe8d20f4..c31fe9a2 100644 --- a/lib/PublicInbox/XapHelper.pm +++ b/lib/PublicInbox/XapHelper.pm @@ -17,7 +17,7 @@ use autodie qw(open); use POSIX qw(:signal_h); use Fcntl qw(LOCK_UN LOCK_EX); my $X = \%PublicInbox::Search::X; -our (%SRCH, %WORKERS, $parent_pid, $alive, $nworker, $workerset); +our (%SRCH, %WORKERS, $alive, $nworker, $workerset); our $stderr = \*STDERR; # only short options for portability in C++ implementation @@ -177,7 +177,8 @@ sub recv_loop { local $SIG{__WARN__} = sub { print $stderr @_ }; my $rbuf; my $in = \*STDIN; - while (!defined($parent_pid) || getppid == $parent_pid) { + local $SIG{TERM} = sub { undef $in }; + while (defined($in)) { PublicInbox::DS::sig_setmask($workerset); my @fds = $PublicInbox::IPC::recv_cmd->($in, $rbuf, 4096*33); scalar(@fds) or exit(66); # EX_NOINPUT @@ -218,7 +219,6 @@ sub start_worker ($) { if ($pid == 0) { undef %WORKERS; PublicInbox::DS::Reset(); - $SIG{TERM} = sub { $parent_pid = -1 }; $SIG{TTIN} = $SIG{TTOU} = 'IGNORE'; $SIG{CHLD} = 'DEFAULT'; # Xapian may use this recv_loop(); @@ -267,7 +267,6 @@ sub start (@) { for (POSIX::SIGTERM, POSIX::SIGCHLD) { $workerset->delset($_) or die "delset($_): $!"; } - local $parent_pid = $$; my $sig = { TTIN => sub { if ($alive) { diff --git a/lib/PublicInbox/xap_helper.h b/lib/PublicInbox/xap_helper.h index c68202c3..fafb392d 100644 --- a/lib/PublicInbox/xap_helper.h +++ b/lib/PublicInbox/xap_helper.h @@ -72,8 +72,8 @@ assert(ckvar______ == (expect) && "BUG" && __FILE__ && __LINE__); \ } while (0) -static const int sock_fd = STDIN_FILENO; // SOCK_SEQPACKET as stdin :P -static volatile pid_t parent_pid; // may be set in worker sighandler (sigw) +// sock_fd is modified in signal handler, yes, it's SOCK_SEQPACKET +static volatile int sock_fd = STDIN_FILENO; static sigset_t fullset, workerset; static bool alive = true; #if STDERR_ASSIGNABLE @@ -640,6 +640,7 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len) union my_cmsg cmsg = {}; struct msghdr msg = {}; struct iovec iov; + ssize_t r; iov.iov_base = rbuf; iov.iov_len = *len; msg.msg_iov = &iov; @@ -650,25 +651,29 @@ static bool recv_req(struct req *req, char *rbuf, size_t *len) // allow SIGTERM to hit CHECK(int, 0, sigprocmask(SIG_SETMASK, &workerset, NULL)); - ssize_t r = recvmsg(sock_fd, &msg, 0); +again: + r = recvmsg(sock_fd, &msg, 0); if (r == 0) { exit(EX_NOINPUT); /* grandparent went away */ } else if (r < 0) { - if (errno == EINTR) - return false; // retry recv_loop - err(EXIT_FAILURE, "recvmsg"); + switch (errno) { + case EINTR: goto again; + case EBADF: if (sock_fd < 0) exit(0); + // fall-through + default: err(EXIT_FAILURE, "recvmsg"); + } } // success! no signals for the rest of the request/response cycle CHECK(int, 0, sigprocmask(SIG_SETMASK, &fullset, NULL)); *len = r; - if (r > 0 && cmsg.hdr.cmsg_level == SOL_SOCKET && + if (cmsg.hdr.cmsg_level == SOL_SOCKET && cmsg.hdr.cmsg_type == SCM_RIGHTS) { - size_t len = cmsg.hdr.cmsg_len; + size_t clen = cmsg.hdr.cmsg_len; int *fdp = (int *)CMSG_DATA(&cmsg.hdr); size_t i; - for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= len; i++) { + for (i = 0; CMSG_LEN((i + 1) * sizeof(int)) <= clen; i++) { int fd = *fdp++; const char *mode = NULL; int fl = fcntl(fd, F_GETFL); @@ -923,7 +928,7 @@ static void stderr_restore(FILE *tmp_err) static void sigw(int sig) // SIGTERM handler for worker { - parent_pid = -1; // break out of recv_loop + sock_fd = -1; // break out of recv_loop } static void recv_loop(void) // worker process loop @@ -934,7 +939,7 @@ static void recv_loop(void) // worker process loop CHECK(int, 0, sigaction(SIGTERM, &sa, NULL)); - while (!parent_pid || getppid() == parent_pid) { + while (sock_fd == 0) { size_t len = sizeof(rbuf); struct req req = {}; if (!recv_req(&req, rbuf, &len)) @@ -1197,7 +1202,6 @@ int main(int argc, char *argv[]) } CHECK(int, 0, sigdelset(&workerset, SIGTERM)); CHECK(int, 0, sigdelset(&workerset, SIGCHLD)); - parent_pid = getpid(); nworker_hwm = nworker; worker_pids = (pid_t *)calloc(nworker, sizeof(pid_t)); if (!worker_pids) err(EXIT_FAILURE, "calloc");