From 0539775d2400654fb28695110fef88d9f6de3705 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Tue, 14 Feb 2012 10:54:02 -0700 Subject: [PATCH] Always close STDIN if --daemonize. --- lib/Daemon.pm | 19 +++++++++++-------- t/lib/Daemon.t | 40 ++++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/lib/Daemon.pm b/lib/Daemon.pm index 0b8b0a32..acc9e1fb 100644 --- a/lib/Daemon.pm +++ b/lib/Daemon.pm @@ -71,14 +71,17 @@ sub daemonize { $OUTPUT_AUTOFLUSH = 1; - # Only reopen STDIN to /dev/null if it's a tty. It may be a pipe, - # in which case we don't want to break it. - if ( -t STDIN ) { - PTDEBUG && _d('STDIN is a terminal; redirecting from /dev/null'); - close STDIN; - open STDIN, '/dev/null' - or die "Cannot reopen STDIN to /dev/null: $OS_ERROR"; - } + # We used to only reopen STDIN to /dev/null if it's a tty because + # otherwise it may be a pipe, in which case we didn't want to break + # it. However, Perl -t is not reliable. This is true and false on + # various boxes even when the same code is ran, or it depends on if + # the code is ran via cron, Jenkins, etc. Since there should be no + # sane reason to `foo | pt-tool --daemonize` for a tool that reads + # STDIN, we now just always close STDIN. + PTDEBUG && _d('Redirecting STDIN to /dev/null'); + close STDIN; + open STDIN, '/dev/null' + or die "Cannot reopen STDIN to /dev/null: $OS_ERROR"; if ( $self->{log_file} ) { PTDEBUG && _d('Redirecting STDOUT and STDERR to', $self->{log_file}); diff --git a/t/lib/Daemon.t b/t/lib/Daemon.t index 6a35b117..810e03e3 100644 --- a/t/lib/Daemon.t +++ b/t/lib/Daemon.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 22; +use Test::More tests => 21; use Time::HiRes qw(sleep); use Daemon; use OptionParser; @@ -103,8 +103,8 @@ unlike( # ########################################################################## rm_tmp_files(); SKIP: { - skip 'No /proc', 2 unless -d '/proc'; - skip 'No fd in /proc', 2 unless -l "/proc/$PID/0" || -l "/proc/$PID/fd/0"; + skip 'No /proc', 1 unless -d '/proc'; + skip 'No fd in /proc', 1 unless -l "/proc/$PID/0" || -l "/proc/$PID/fd/0"; system("$cmd 1 --daemonize --pid $pid_file --log $log_file"); PerconaTest::wait_for_files($pid_file); @@ -113,29 +113,29 @@ SKIP: { : -l "/proc/$pid/fd/0" ? "/proc/$pid/fd/0" : die "Cannot find fd 0 symlink in /proc/$pid"; PTDEVDEBUG && PerconaTest::_d('pid_file', $pid_file, - 'pid', $pid, 'proc_fd_0', $proc_fd_0, `ls -l $proc_fd_0`, `lsof -p $pid`); + 'pid', $pid, 'proc_fd_0', $proc_fd_0, `ls -l $proc_fd_0`); my $stdin = readlink $proc_fd_0; is( $stdin, '/dev/null', - 'Reopens STDIN to /dev/null if not piped', + 'Reopens STDIN to /dev/null' ); - sleep 1; - system("echo foo | $cmd 1 --daemonize --pid $pid_file --log $log_file"); - PerconaTest::wait_for_files($pid_file, $log_file); - chomp($pid = `cat $pid_file`); - $proc_fd_0 = -l "/proc/$pid/0" ? "/proc/$pid/0" - : -l "/proc/$pid/fd/0" ? "/proc/$pid/fd/0" - : die "Cannot find fd 0 symlink in /proc/$pid"; - PTDEVDEBUG && PerconaTest::_d('pid_file', $pid_file, - 'pid', $pid, 'proc_fd_0', $proc_fd_0, `ls -l $proc_fd_0`, `lsof -p $pid`); - $stdin = readlink $proc_fd_0; - like( - $stdin, - qr/pipe/, - 'Does not reopen STDIN to /dev/null when piped', - ); +# sleep 1; +# system("echo foo | $cmd 1 --daemonize --pid $pid_file --log $log_file"); +# PerconaTest::wait_for_files($pid_file, $log_file); +# chomp($pid = `cat $pid_file`); +# $proc_fd_0 = -l "/proc/$pid/0" ? "/proc/$pid/0" +# : -l "/proc/$pid/fd/0" ? "/proc/$pid/fd/0" +# : die "Cannot find fd 0 symlink in /proc/$pid"; +# PTDEVDEBUG && PerconaTest::_d('pid_file', $pid_file, +# 'pid', $pid, 'proc_fd_0', $proc_fd_0, `ls -l $proc_fd_0`); +# $stdin = readlink $proc_fd_0; +# like( +# $stdin, +# qr/pipe/, +# 'Does not reopen STDIN to /dev/null when piped', +# ); sleep 1; };