[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Sun Mar 4 03:52:03 MST 2012


The branch, master has been updated
       via  48e9d7e selftest: Rework Samba3.pm process termination
       via  a37990c selftest: Rework Samba4.pm process termination.
       via  e495118 selftest: Fix waitpid termination test
       via  d715e2c selftest: Use fork()/exec() rather than system()
       via  769cee4 s3-winbindd: Add stdin handler for winbind
       via  807f5f1 s3-nmbd: Add stdin handler for nmbd
       via  b07d504 change low FDs are handled in Samba
      from  679bbd0 s3: don't replace the error message if already defined

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 48e9d7efce1be3b09ed19b3d9a5bf3a3df3800e7
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sun Mar 4 18:34:34 2012 +1100

    selftest: Rework Samba3.pm process termination
    
    We now store the timelimit child PID in memory, and confirm that the child has exited
    with both waitpid() and kill(0, $pid).
    
    By calling Samba::cleanup_child() we get exit status information.
    
    Andrew Bartlett
    
    Autobuild-User: Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date: Sun Mar  4 11:51:12 CET 2012 on sn-devel-104

commit a37990c5c15e8077c5466ed139044ed26bae8c5e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sun Mar 4 18:32:44 2012 +1100

    selftest: Rework Samba4.pm process termination.
    
    We now double-check the waitpid() result with kill(0, $pid)
    
    We now also send a SIGTERM, then a SIGKILL.
    
    Andrew Bartlett

commit e495118aaba20d05e6da937040de4f82b600b70b
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sun Mar 4 18:10:35 2012 +1100

    selftest: Fix waitpid termination test

commit d715e2c2948cf642e725bee56f1f43e2395a2025
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Sun Mar 4 17:30:45 2012 +1100

    selftest: Use fork()/exec() rather than system()
    
    This follows the Samba3.pm model for starting child processes is to
    use fork()/exec().  This reduces the number of processes being created
    by selftest.pl, and gives us more information about the child process
    and the running state in the parent.
    
    Andrew Bartlett

commit 769cee44a2ed1b68cb757246efd72d63aa36a4d0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Mar 2 18:22:10 2012 +1100

    s3-winbindd: Add stdin handler for winbind
    
    This will help avoid runaway processes in the test env, particularly when
    the whole selftest.pl is killed.
    
    Andrew Bartlett

commit 807f5f1a8b02176e03b00f4415c0cae1927b44a4
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Mar 2 18:21:09 2012 +1100

    s3-nmbd: Add stdin handler for nmbd
    
    This will help avoid runaway processes in the test env, particularly
    when the whole selftest.pl is killed.
    
    Andrew Bartlett

commit b07d504ca4e476d492beb5552344070e4f96464a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Mar 2 19:32:56 2012 +1100

    change low FDs are handled in Samba
    
    We now only close fds 0, 1, 2 when we are a forked daemon, and take
    care not to close a file descriptor that we might need for foreground
    stdin monitoring.
    
    This should fix stdout logging in the lsa and epmapper deamons (ie in
    make test).
    
    Andrew Bartlett

-----------------------------------------------------------------------

Summary of changes:
 lib/util/become_daemon.c          |   22 ++++---
 lib/util/debug.c                  |   11 +++-
 lib/util/samba_util.h             |    2 +-
 selftest/target/Samba.pm          |   20 ++++++
 selftest/target/Samba3.pm         |   38 +++++++-----
 selftest/target/Samba4.pm         |  123 ++++++++++++++++---------------------
 source3/nmbd/nmbd.c               |   33 ++++++++++
 source3/printing/spoolssd.c       |    3 -
 source3/rpc_server/epmd.c         |    3 -
 source3/rpc_server/lsasd.c        |    3 -
 source3/smbd/server.c             |   12 +---
 source3/winbindd/winbindd.c       |   48 ++++++++++++++-
 source3/winbindd/winbindd_proto.h |    3 +-
 13 files changed, 203 insertions(+), 118 deletions(-)


Changeset truncated at 500 lines:

diff --git a/lib/util/become_daemon.c b/lib/util/become_daemon.c
index 2af1631..4c1d29e 100644
--- a/lib/util/become_daemon.c
+++ b/lib/util/become_daemon.c
@@ -29,14 +29,16 @@
  Close the low 3 fd's and open dev/null in their place.
 ********************************************************************/
 
-_PUBLIC_ void close_low_fds(bool stderr_too)
+_PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too)
 {
 #ifndef VALGRIND
 	int fd;
 	int i;
 
-	close(0);
-	close(1);
+	if (stdin_too)
+		close(0);
+	if (stdout_too)
+		close(1);
 
 	if (stderr_too)
 		close(2);
@@ -44,6 +46,10 @@ _PUBLIC_ void close_low_fds(bool stderr_too)
 	/* try and use up these file descriptors, so silly
 		library routines writing to stdout etc won't cause havoc */
 	for (i=0;i<3;i++) {
+		if (i == 0 && !stdin_too)
+			continue;
+		if (i == 1 && !stdout_too)
+			continue;
 		if (i == 2 && !stderr_too)
 			continue;
 
@@ -87,9 +93,9 @@ _PUBLIC_ void become_daemon(bool do_fork, bool no_process_group, bool log_stdout
 	}
 #endif /* HAVE_SETSID */
 
-	if (!log_stdout) {
-		/* Close fd's 0,1,2. Needed if started by rsh */
-		close_low_fds(false);  /* Don't close stderr, let the debug system
-					  attach it to the logfile */
-	}
+	/* Close fd's 0,1,2 as appropriate. Needed if started by rsh. */
+	/* stdin must be open if we do not fork, for monitoring for
+	 * close.  stdout must be open if we are logging there, and we
+	 * never close stderr (but debug might dup it onto a log file) */
+	close_low_fds(do_fork, !log_stdout, false);
 }
diff --git a/lib/util/debug.c b/lib/util/debug.c
index a638851..a7e2a0f 100644
--- a/lib/util/debug.c
+++ b/lib/util/debug.c
@@ -592,9 +592,14 @@ bool reopen_logs_internal(void)
 	(void)umask(oldumask);
 
 	/* Take over stderr to catch output into logs */
-	if (state.fd > 0 && dup2(state.fd, 2) == -1) {
-		close_low_fds(true); /* Close stderr too, if dup2 can't point it
-					at the logfile */
+	if (state.fd > 0) {
+		if (dup2(state.fd, 2) == -1) {
+			/* Close stderr too, if dup2 can't point it -
+			   at the logfile.  There really isn't much
+			   that can be done on such a fundemental
+			   failure... */
+			close_low_fds(false, false, true);
+		}
 	}
 
 	state.reopening_logs = false;
diff --git a/lib/util/samba_util.h b/lib/util/samba_util.h
index a0989d5..13fe831 100644
--- a/lib/util/samba_util.h
+++ b/lib/util/samba_util.h
@@ -824,7 +824,7 @@ _PUBLIC_ int idr_remove(struct idr_context *idp, int id);
 /**
  Close the low 3 fd's and open dev/null in their place
 **/
-_PUBLIC_ void close_low_fds(bool stderr_too);
+_PUBLIC_ void close_low_fds(bool stdin_too, bool stdout_too, bool stderr_too);
 
 /**
  Become a daemon, discarding the controlling terminal.
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 06b6472..1422603 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -8,6 +8,7 @@ package Samba;
 use strict;
 use target::Samba3;
 use target::Samba4;
+use POSIX;
 
 sub new($$$$$) {
 	my ($classname, $bindir, $binary_mapping,$ldap, $srcdir, $server_maxtime) = @_;
@@ -172,4 +173,23 @@ sub get_interface($)
 
     return $interfaces{$netbiosname};
 }
+
+sub cleanup_child($$)
+{
+    my ($pid, $name) = @_;
+    my $childpid = waitpid($pid, WNOHANG);
+    if ($childpid == 0) {
+    } elsif ($childpid < 0) {
+	printf STDERR "%s child process %d isn't here any more\n",
+	return $childpid;
+    }
+    elsif ($? & 127) {
+	printf STDERR "%s child process %d, died with signal %d, %s coredump\n",
+	$name, $childpid, ($? & 127),  ($? & 128) ? 'with' : 'without';
+    } else {
+	printf STDERR "%s child process %d exited with value %d\n", $name, $childpid, $? >> 8;
+    }
+    return $childpid;
+}
+
 1;
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index c9ad7d3..87763aa 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -48,19 +48,22 @@ sub teardown_env($$)
 	# This should cause smbd to terminate gracefully
 	close($envvars->{STDIN_PIPE});
 
-	my $smbdpid = read_pid($envvars, "smbd");
-	my $nmbdpid = read_pid($envvars, "nmbd");
-	my $winbinddpid = read_pid($envvars, "winbindd");
-
-	until (kill(0, $smbdpid, $nmbdpid, $winbinddpid) == 0) {
-	    my $childpid = waitpid(-1, WNOHANG);
-	    # This should give it time to write out the gcov data
+	my $smbdpid = $envvars->{SMBD_TL_PID};
+	my $nmbdpid = $envvars->{NMBD_TL_PID};
+	my $winbinddpid = $envvars->{WINBINDD_TL_PID};
+
+	# This should give it time to write out the gcov data
+	until ($count > 20) {
+	    if (Samba::cleanup_child($smbdpid, "smbd") == -1
+		&& Samba::cleanup_child($nmbdpid, "nmbd") == -1
+		&& Samba::cleanup_child($winbinddpid, "winbindd") == -1) {
+		last;
+	    }
 	    sleep(1);
 	    $count++;
-	    last if $childpid == 0 or $count > 20;
 	}
 
-	if ($count <= 20) {
+	if ($count <= 20 && kill(0, $smbdpid, $nmbdpid, $winbinddpid) == 0) {
 	    return;
 	}
 
@@ -69,15 +72,17 @@ sub teardown_env($$)
 	$self->stop_sig_term($winbinddpid);
 
 	$count = 0;
-	until (kill(0, $smbdpid, $nmbdpid, $winbinddpid) == 0) {
-	    # if no process sucessfully signalled, then we are done
-	    my $childpid = waitpid(-1, WNOHANG);
+	until ($count > 10) {
+	    if (Samba::cleanup_child($smbdpid, "smbd") == -1
+		&& Samba::cleanup_child($nmbdpid, "nmbd") == -1
+		&& Samba::cleanup_child($winbinddpid, "winbindd") == -1) {
+		last;
+	    }
 	    sleep(1);
 	    $count++;
-	    last if $childpid == 0 or $count > 20;
 	}
-	
-	if ($count <= 10) {
+
+	if ($count <= 10 && kill(0, $smbdpid, $nmbdpid, $winbinddpid) == 0) {
 	    return;
 	}
 
@@ -591,6 +596,7 @@ sub check_or_start($$$$$) {
 
 		exec(@preargs, Samba::bindir_path($self, "nmbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start nmbd: $!");
 	}
+	$env_vars->{NMBD_TL_PID} = $pid;
 	write_pid($env_vars, "nmbd", $pid);
 	print "DONE\n";
 
@@ -642,6 +648,7 @@ sub check_or_start($$$$$) {
 
 		exec(@preargs, Samba::bindir_path($self, "winbindd"), "-F", "--no-process-group", "--stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start winbindd: $!");
 	}
+	$env_vars->{WINBINDD_TL_PID} = $pid;
 	write_pid($env_vars, "winbindd", $pid);
 	print "DONE\n";
 
@@ -689,6 +696,7 @@ sub check_or_start($$$$$) {
 
 		exec(@preargs, Samba::bindir_path($self, "smbd"), "-F", "--no-process-group", "--log-stdout", "-s", $env_vars->{SERVERCONFFILE}, @optargs) or die("Unable to start smbd: $!");
 	}
+	$env_vars->{SMBD_TL_PID} = $pid;
 	write_pid($env_vars, "smbd", $pid);
 	print "DONE\n";
 
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index b79e29f..8e27620 100644
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -85,18 +85,18 @@ sub slapd_stop($$)
 
 sub check_or_start($$$)
 {
-	my ($self, $env_vars, $process_model) = @_;
-	return 0 if ( -p $env_vars->{SAMBA_TEST_FIFO});
+        my ($self, $env_vars, $process_model) = @_;
 
-	unlink($env_vars->{SAMBA_TEST_FIFO});
-	POSIX::mkfifo($env_vars->{SAMBA_TEST_FIFO}, 0700);
-	unlink($env_vars->{SAMBA_TEST_LOG});
-	
-	my $pwd = `pwd`;
-	print "STARTING SAMBA for $ENV{ENVNAME}\n";
+	return 0 if $self->check_env($env_vars);
+
+	# use a pipe for stdin in the child processes. This allows
+	# those processes to monitor the pipe for EOF to ensure they
+	# exit when the test script exits
+	pipe(STDIN_READER, $env_vars->{STDIN_PIPE});
+
+	print "STARTING SAMBA...";
 	my $pid = fork();
 	if ($pid == 0) {
-		open STDIN, $env_vars->{SAMBA_TEST_FIFO};
 		# we want out from samba to go to the log file, but also
 		# to the users terminal when running 'make test' on the command
 		# line. This puts it on stderr on the terminal
@@ -105,13 +105,9 @@ sub check_or_start($$$)
 
 		SocketWrapper::set_default_iface($env_vars->{SOCKET_WRAPPER_DEFAULT_IFACE});
 
-		my $valgrind = "";
-		if (defined($ENV{SAMBA_VALGRIND})) {
-		    $valgrind = $ENV{SAMBA_VALGRIND};
-		}
-
 		$ENV{KRB5_CONFIG} = $env_vars->{KRB5_CONFIG};
 		$ENV{WINBINDD_SOCKET_DIR} = $env_vars->{WINBINDD_SOCKET_DIR};
+		$ENV{NMBD_SOCKET_DIR} = $env_vars->{NMBD_SOCKET_DIR};
 
 		$ENV{NSS_WRAPPER_PASSWD} = $env_vars->{NSS_WRAPPER_PASSWD};
 		$ENV{NSS_WRAPPER_GROUP} = $env_vars->{NSS_WRAPPER_GROUP};
@@ -119,46 +115,25 @@ sub check_or_start($$$)
 
 		$ENV{UID_WRAPPER} = "1";
 
-		# Start slapd before samba, but with the fifo on stdin
-		if (defined($self->{ldap})) {
-		 	unless($self->slapd_start($env_vars)) {
-				warn("couldn't start slapd (main run)");
-				return undef;
-			}
-		}
-
-		my $optarg = "";
-		$optarg = "--maximum-runtime=$self->{server_maxtime}";
+		$ENV{MAKE_TEST_BINARY} = Samba::bindir_path($self, "samba");
+		my @preargs = ();
+		my @optargs = ();
 		if (defined($ENV{SAMBA_OPTIONS})) {
-			$optarg.= " $ENV{SAMBA_OPTIONS}";
-		}
-		my $samba =  Samba::bindir_path($self, "samba");
-
-		chomp($pwd);
-		my $cmdline = "$valgrind ${pwd}/$samba $optarg $env_vars->{CONFIGURATION} -M $process_model -i";
-		my $ret = system("$cmdline");
-		if ($ret == -1) {
-			print "Unable to start $cmdline: $ret: $!\n";
-			exit 1;
+			@optargs = split(/ /, $ENV{SAMBA_OPTIONS});
 		}
-		my $exit = ($ret >> 8);
-		unlink($env_vars->{SAMBA_TEST_FIFO});
-		if ($ret == 0) {
-			print "$samba exited with no error\n";
-			exit 0;
-		} elsif ( $ret & 127 ) {
-			print "$samba got signal ".($ret & 127)." and exits with $exit!\n";
-		} else {
-			print "$samba failed with status $exit!\n";
+		if(defined($ENV{SAMBA_VALGRIND})) {
+			@preargs = split(/ /,$ENV{SAMBA_VALGRIND});
 		}
-		if ($exit == 0) {
-			$exit = -1;
-		}
-		exit $exit;
+
+		close($env_vars->{STDIN_PIPE});
+		open STDIN, ">&", \*STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
+
+		exec(@preargs, Samba::bindir_path($self, "samba"), "-M", $process_model, "-i", "--maximum-runtime=$self->{server_maxtime}", $env_vars->{CONFIGURATION}, @optargs) or die("Unable to start samba: $!");
 	}
+	$env_vars->{SAMBA_PID} = $pid;
 	print "DONE\n";
 
-	open($env_vars->{STDIN_PIPE}, ">$env_vars->{SAMBA_TEST_FIFO}");
+	close(STDIN_READER);
 
 	return $pid;
 }
@@ -1357,34 +1332,44 @@ sub teardown_env($$)
 	# This should cause samba to terminate gracefully
 	close($envvars->{STDIN_PIPE});
 
-	if (open(IN, "<$envvars->{PIDDIR}/samba.pid")) {
-		$pid = <IN>;
-		close(IN);
-		my $count = 0;
+	$pid = $envvars->{SAMBA_PID};
+	my $count = 0;
+	my $childpid;
+
+	# This should give it time to write out the gcov data
+	until ($count > 20) {
+	    if (Samba::cleanup_child($pid, "samba") == -1) {
+		last;
+	    }
+	    sleep(1);
+	    $count++;
+	}
 
-		until (kill(0, $pid) == 0) {
-		    my $childpid = waitpid(-1, WNOHANG);
-	
-		    # This should give it time to write out the gcov data
-		    sleep(1);
-		    $count++;
-		    last if $childpid == 0 or $count > 20;
-		}
+	if ($count <= 20 && kill(0, $pid) == 0) {
+	    return;
+	}
 
-		# If it is still around, kill it
-		if ($count > 20) {
-			print "server process $pid took more than $count seconds to exit, killing\n";
-			kill 9, $pid;
-		}
+	kill "TERM", $pid;
+
+	until ($count > 20) {
+	    if (Samba::cleanup_child($pid, "samba") == -1) {
+		last;
+	    }
+	    sleep(1);
+	    $count++;
 	}
 
-	my $failed = $? >> 8;
+	# If it is still around, kill it
+	if ($count > 20 && kill(0, $pid) == 0) {
+	    warn "server process $pid took more than $count seconds to exit, killing\n";
+	    kill 9, $pid;
+	}
 
 	$self->slapd_stop($envvars) if ($self->{ldap});
 
 	print $self->getlog_env($envvars);
 
-	return $failed;
+	return;
 }
 
 sub getlog_env($$)
@@ -1411,9 +1396,9 @@ sub check_env($$)
 {
 	my ($self, $envvars) = @_;
 
-	my $childpid = waitpid(-1, WNOHANG);
+	my $childpid = Samba::cleanup_child($envvars->{SAMBA_PID}, "samba");
 
-	return (-p $envvars->{SAMBA_TEST_FIFO});
+	return ($childpid == 0);
 }
 
 sub setup_env($$$)
diff --git a/source3/nmbd/nmbd.c b/source3/nmbd/nmbd.c
index 35a9a31..7bd4fac 100644
--- a/source3/nmbd/nmbd.c
+++ b/source3/nmbd/nmbd.c
@@ -88,6 +88,24 @@ static void nmbd_sig_term_handler(struct tevent_context *ev,
 	terminate(msg);
 }
 
+/*
+  handle stdin becoming readable when we are in --foreground mode
+ */
+static void nmbd_stdin_handler(struct tevent_context *ev,
+			       struct tevent_fd *fde,
+			       uint16_t flags,
+			       void *private_data)
+{
+	char c;
+	if (read(0, &c, 1) != 1) {
+		struct messaging_context *msg = talloc_get_type_abort(
+			private_data, struct messaging_context);
+		
+		DEBUG(0,("EOF on stdin\n"));
+		terminate(msg);
+	}
+}
+
 static bool nmbd_setup_sig_term_handler(struct messaging_context *msg)
 {
 	struct tevent_signal *se;
@@ -105,6 +123,19 @@ static bool nmbd_setup_sig_term_handler(struct messaging_context *msg)
 	return true;
 }
 
+static bool nmbd_setup_stdin_handler(struct messaging_context *msg, bool foreground)
+{
+	if (foreground) {
+		/* if we are running in the foreground then look for
+		   EOF on stdin, and exit if it happens. This allows
+		   us to die if the parent process dies
+		*/
+		tevent_add_fd(nmbd_event_context(), nmbd_event_context(), 0, TEVENT_FD_READ, nmbd_stdin_handler, msg);
+	}
+
+	return true;
+}
+
 static void msg_reload_nmbd_services(struct messaging_context *msg,
 				     void *private_data,
 				     uint32_t msg_type,
@@ -918,6 +949,8 @@ static bool open_sockets(bool isdaemon, int port)
 
 	if (!nmbd_setup_sig_term_handler(msg))
 		exit(1);
+	if (!nmbd_setup_stdin_handler(msg, !Fork))
+		exit(1);
 	if (!nmbd_setup_sig_hup_handler(msg))
 		exit(1);
 
diff --git a/source3/printing/spoolssd.c b/source3/printing/spoolssd.c
index 9a5d1b0..5775505 100644
--- a/source3/printing/spoolssd.c
+++ b/source3/printing/spoolssd.c
@@ -660,9 +660,6 @@ pid_t start_spoolssd(struct tevent_context *ev_ctx,
 		return pid;
 	}
 
-	/* child */
-	close_low_fds(false);
-
 	status = reinit_after_fork(msg_ctx,
 				   ev_ctx,
 				   true);
diff --git a/source3/rpc_server/epmd.c b/source3/rpc_server/epmd.c
index 0d5e6ec..c28d857 100644
--- a/source3/rpc_server/epmd.c
+++ b/source3/rpc_server/epmd.c
@@ -159,9 +159,6 @@ void start_epmd(struct tevent_context *ev_ctx,
 		return;
 	}
 
-	/* child */
-	close_low_fds(false);
-
 	status = reinit_after_fork(msg_ctx,
 				   ev_ctx,
 				   true);
diff --git a/source3/rpc_server/lsasd.c b/source3/rpc_server/lsasd.c
index 575b863..416a3b3 100644
--- a/source3/rpc_server/lsasd.c
+++ b/source3/rpc_server/lsasd.c
@@ -875,9 +875,6 @@ void start_lsasd(struct tevent_context *ev_ctx,
 		return;
 	}
 
-	/* child */
-	close_low_fds(false);
-
 	/* save the parent process id so the children can use it later */
 	parent_id = procid_self();
 
diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 0fb7d16..cab23bc 100644
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -504,12 +504,6 @@ static void smbd_accept_connection(struct tevent_context *ev,
 		 * them, counting worker smbds. */
 		CatchChild();
 
-		/* close our standard file
-		   descriptors */
-		if (!debug_get_output_is_stdout()) {
-			close_low_fds(False); /* Don't close stderr */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list