user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH] daemon: fix various permissions + daemon issues
@ 2015-09-25  8:43  7% Eric Wong
  0 siblings, 0 replies; 1+ results
From: Eric Wong @ 2015-09-25  8:43 UTC (permalink / raw)
  To: meta

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


^ permalink raw reply related	[relevance 7%]

Results 1-1 of 1 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2015-09-25  8:43  7% [PATCH] daemon: fix various permissions + daemon issues Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).