From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-2.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00, RP_MATCHES_RCVD shortcircuit=no autolearn=unavailable version=3.3.2 X-Original-To: meta@public-inbox.org Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 7A6ED1FA66 for ; Fri, 25 Sep 2015 08:43:15 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] daemon: fix various permissions + daemon issues Date: Fri, 25 Sep 2015 08:43:15 +0000 Message-Id: <20150925084315.12182-1-e@80x24.org> List-Id: When using user-switching in a single process, we must be careful to not inadvertantly create new Msgmap sqlite3 files. We must also ensure we set proper permissions on any files we create. Additionally, our refactoring was broken as we failed to actually daemonize or preserve the parent FD in a worker process. Finally, default to one worker process since our code may be fatally broken and it's nice to be able to scale to multiple cores via SIGTTIN if needed. --- lib/PublicInbox/Daemon.pm | 52 +++++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index 81ff21c..9e177f2 100644 --- a/lib/PublicInbox/Daemon.pm +++ b/lib/PublicInbox/Daemon.pm @@ -14,12 +14,13 @@ require PublicInbox::Listener; my @CMD; my $set_user; my (@cfg_listen, $stdout, $stderr, $group, $user, $pid_file, $daemonize); -my $worker_processes = 0; +my $worker_processes = 1; my @listeners; my %pids; my %listener_names; my $reexec_pid; my $cleanup; +my ($uid, $gid); END { $cleanup->() if $cleanup }; sub daemon_prepare ($) { @@ -76,8 +77,7 @@ sub daemonize () { require Net::Server::Daemonize; Net::Server::Daemonize::check_pid_file($pid_file) if defined $pid_file; - my $uid = Net::Server::Daemonize::get_uid($user) if defined $user; - my $gid; + $uid = Net::Server::Daemonize::get_uid($user) if defined $user; if (defined $group) { $gid = Net::Server::Daemonize::get_gid($group); $gid = (split /\s+/, $gid)[0]; @@ -105,10 +105,7 @@ sub daemonize () { exit if $pid; } if (defined $pid_file) { - Net::Server::Daemonize::create_pid_file($pid_file); - if ($uid and !chown($uid, $gid, $pid_file)) { - warn "could not chown $pid_file: $!\n"; - } + write_pid($pid_file); my $unlink_pid = $$; $cleanup = sub { unlink_pid_file_safe_ish($unlink_pid, $pid_file); @@ -146,10 +143,14 @@ sub reopen_logs { if ($stdout) { open STDOUT, '>>', $stdout or warn "failed to redirect stdout to $stdout: $!\n"; + STDOUT->autoflush(1); + do_chown($stdout); } if ($stderr) { open STDERR, '>>', $stderr or warn "failed to redirect stderr to $stderr: $!\n"; + STDERR->autoflush(1); + do_chown($stderr); } } @@ -201,7 +202,7 @@ sub upgrade () { } unlink_pid_file_safe_ish($$, $pid_file); $pid_file .= '.oldbin'; - Net::Server::Daemonize::create_pid_file($pid_file); + write_pid($pid_file); } my ($pid, $err) = do_fork(); unless (defined $pid) { @@ -253,7 +254,7 @@ sub upgrade_aborted ($) { $file =~ s/\.oldbin\z// or die "BUG: no '.oldbin' suffix in $file\n"; unlink_pid_file_safe_ish($$, $pid_file); $pid_file = $file; - eval { Net::Server::Daemonize::create_pid_file($pid_file) }; + eval { write_pid($pid_file) }; warn $@, "\n" if $@; } @@ -284,10 +285,8 @@ sub unlink_pid_file_safe_ish ($$) { } } -sub master_loop ($) { - my ($refresh) = @_; +sub master_loop { pipe(my ($p0, $p1)) or die "failed to create parent-pipe: $!\n"; - my %pwatch = ( fileno($p0) => sub { kill('TERM', $$) } ); pipe(my ($r, $w)) or die "failed to create self-pipe: $!\n"; IO::Handle::blocking($w, 0); my $set_workers = $worker_processes; @@ -316,7 +315,6 @@ sub master_loop ($) { $worker_processes = 0; } elsif ($s eq 'HUP') { $worker_processes = $set_workers; - $refresh->(); kill_workers($s); } elsif ($s eq 'TTIN') { if ($set_workers > $worker_processes) { @@ -346,9 +344,7 @@ sub master_loop ($) { warn "failed to fork worker[$i]: $err\n"; } elsif ($pid == 0) { $set_user->() if $set_user; - close($_) for ($w, $r, $p1); - Danga::Socket->AddOtherFds(%pwatch); - return; # run normal work code + return $p0; # run normal work code } else { warn "PID=$pid is worker[$i]\n"; $pids{$pid} = $i; @@ -362,27 +358,47 @@ sub master_loop ($) { sub daemon_loop ($$) { my ($refresh, $post_accept) = @_; - $refresh->(); # load config before forking to save work + my $parent_pipe; if ($worker_processes > 0) { - master_loop($refresh); # returns if in child process + $parent_pipe = master_loop(); # returns if in child process + my $fd = fileno($parent_pipe); + Danga::Socket->AddOtherFds($fd => sub { kill('TERM', $$) } ); } else { + reopen_logs(); $set_user->() if $set_user; $SIG{USR2} = sub { worker_quit() if upgrade() }; } + $uid = $gid = undef; reopen_logs(); + $refresh->(); $SIG{QUIT} = $SIG{INT} = $SIG{TERM} = *worker_quit; $SIG{USR1} = *reopen_logs; $SIG{HUP} = $refresh; # this calls epoll_create: PublicInbox::Listener->new($_, $post_accept) for @listeners; Danga::Socket->EventLoop; + $parent_pipe = undef; } sub daemon_run ($$$) { my ($default, $refresh, $post_accept) = @_; daemon_prepare($default); + daemonize(); daemon_loop($refresh, $post_accept); } +sub do_chown ($) { + my ($path) = @_; + if (defined $uid and !chown($uid, $gid, $path)) { + warn "could not chown $path: $!\n"; + } +} + +sub write_pid ($) { + my ($path) = @_; + Net::Server::Daemonize::create_pid_file($path); + do_chown($path); +} + 1; -- EW