From dcee95db6431f313f7fd1b2408390f3ae3718568 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 25 Sep 2015 08:21:43 +0000 Subject: daemon: fix various permissions + daemon issues 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(-) (limited to 'lib/PublicInbox/Daemon.pm') diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm index 81ff21cd..9e177f2e 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; -- cgit v1.2.3-24-ge0c7