Imagine situation when ps will be writing into pipe, when suddenly pipe closed by other process. 'ps' will receive receive SIGPIPE and crash. That is easy to reproduce, for example 'ps | head' I suggest to register signal handler for SIGPIPE in 'ps' and exit gracefully if SIGPIPE received.
(In reply to comment #0) > Imagine situation when ps will be writing into pipe, when suddenly pipe closed > by other process. 'ps' will receive receive SIGPIPE and crash. > > That is easy to reproduce, for example 'ps | head' # busybox ps | head -1 PID USER TIME COMMAND And? > I suggest to register signal handler for SIGPIPE in 'ps' and exit gracefully if > SIGPIPE received. I don't understand what's the problem with current behavior.
Ok, let me explain this issue in more detail. Try to run command: strace -e trace=signal busybox ps | head -n 1 --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=4385, si_uid=1000} --- +++ killed by SIGPIPE +++ As you can see ps got killed by signal, in normal circumstances this doesn't do much effect and can be easily ignored. However if we working on Android system there a small daemon called 'debuggerd' which collects crash dump information about all crashed programs, including programs killed by signals. So crashed ps triggers error handling and bug-reporting mechanism in Android system, which as i believe shouldn't happen.
(In reply to comment #2) > Ok, let me explain this issue in more detail. Try to run command: > > strace -e trace=signal busybox ps | head -n 1 > --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=4385, si_uid=1000} --- > +++ killed by SIGPIPE +++ This is intended. Unix-like OSes have SIGPIPE because it's impractical to expect _every_ program which prints to stdout to correctly handle EPIPE write error. People will forget to do that, making these constructs: <program> | head <program> | less not terminating when second member of the pipe exits. > However if we working on Android system > there a small daemon called 'debuggerd' which collects crash dump information > about all crashed programs, including programs killed by signals. > > So crashed ps triggers error handling and bug-reporting mechanism in Android > system, which as i believe shouldn't happen. This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few other signals (SIGINT, SIGQUIT, SIGABRT).
(In reply to comment #3) > This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few > other signals (SIGINT, SIGQUIT, SIGABRT). Main purpose of debuggerd is to handle unexpected failures in applications and if ps crashed with SIGPIPE, then it accounts as 'unexpected'. If ignore certain signals as you suggested can lead to miss of some 'real' crashes. According to article https://kobablog.wordpress.com/2011/05/12/debuggerd-of-android/ . If you don't want to call debuggerd at SIGPIPE then you should change signal handler for SIGPIPE to default handler. Also, SIGPIPE handling implemented in mature project like procps: http://procps.cvs.sourceforge.net/viewvc/procps/procps/ps/display.c?revision=1.32&view=markup
(In reply to comment #4) > (In reply to comment #3) > > This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few > > other signals (SIGINT, SIGQUIT, SIGABRT). > > Main purpose of debuggerd is to handle unexpected failures in applications and > if ps crashed with SIGPIPE, then it accounts as 'unexpected'. I disagree. It is an expected failure.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few > > > other signals (SIGINT, SIGQUIT, SIGABRT). > > > > Main purpose of debuggerd is to handle unexpected failures in applications and > > if ps crashed with SIGPIPE, then it accounts as 'unexpected'. > > I disagree. It is an expected failure. So, if you expect it, why not to set signal handler properly?
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > (In reply to comment #3) > > > > This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few > > > > other signals (SIGINT, SIGQUIT, SIGABRT). > > > > > > Main purpose of debuggerd is to handle unexpected failures in applications and > > > if ps crashed with SIGPIPE, then it accounts as 'unexpected'. > > > > I disagree. It is an expected failure. > > So, if you expect it, why not to set signal handler properly? I mean: it is expected for processes to die from SIGPIPE. It is not a bug. It was happening in Unix from its early days. debuggerd (supposedly) wants to catch BUGS, not normal behavior. Therefore it needs to catch signals which indicate bugs: SIGSEGV, SIGBUS, SIGFPE, SIGILL. _Not_ SIGPIPE.
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > (In reply to comment #3) > > > > > This is a bug in debuggerd. It needs to ignore deaths from SIGPIPE and a few > > > > > other signals (SIGINT, SIGQUIT, SIGABRT). > > > > > > > > Main purpose of debuggerd is to handle unexpected failures in applications and > > > > if ps crashed with SIGPIPE, then it accounts as 'unexpected'. > > > > > > I disagree. It is an expected failure. > > > > So, if you expect it, why not to set signal handler properly? > > I mean: it is expected for processes to die from SIGPIPE. It is not a bug. It > was happening in Unix from its early days. > > debuggerd (supposedly) wants to catch BUGS, not normal behavior. Therefore it > needs to catch signals which indicate bugs: SIGSEGV, SIGBUS, SIGFPE, SIGILL. > _Not_ SIGPIPE. SIGPIPE may indicate bug in some cases, but in some it is expected. So If ps is expected SIGPIPE, why not to handle it properly? Why do you leave signal handler for SIGPIPE which would trigger debuggerd dump reporting process?
Look, a horrible bug in coreutils cat: $ strace cat </dev/zero | sleep 1 execve("/usr/bin/cat", ["cat"], [/* 52 vars */]) = 0 brk(0) = 0x1071000 ... fstat(1, {st_mode=S_IFIFO|0600, st_size=0, ...}) = 0 fstat(0, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 5), ...}) = 0 fadvise64(0, 0, 0, POSIX_FADV_SEQUENTIAL) = 0 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = -1 EPIPE (Broken pipe) --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=5851, si_uid=0} --- +++ killed by SIGPIPE +++ Oh, check this out, bash is buggy: $ yes '111111111111111111111111' | strace sh -c 'while read line; do echo "$line"; done' | sleep 1 ... ... ... read(0, "1", 1) = 1 read(0, "1", 1) = 1 read(0, "\n", 1) = 1 write(1, "111111111111111111111111\n", 25) = -1 EPIPE (Broken pipe) --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=5872, si_uid=0} --- +++ killed by SIGPIPE +++ $ Oops, a bug in GNU sed: $ yes '111111111111111111111111' | strace sed 's/qwe/rty/' | sleep 1 execve("/usr/bin/sed", ["sed", "s/qwe/rty/"], [/* 52 vars */]) = 0 ... ... ... mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f0a4a073000 read(0, "111\n111111111111111111111111\n111"..., 4096) = 4096 write(1, "111111111111111111111111\n1111111"..., 4096) = 4096 read(0, "1111111\n111111111111111111111111"..., 4096) = 4096 write(1, "111\n111111111111111111111111\n111"..., 4096) = 4096 read(0, "11111111111\n11111111111111111111"..., 4096) = 4096 write(1, "1111111\n111111111111111111111111"..., 4096) = 4096 read(0, "111111111111111\n1111111111111111"..., 4096) = 4096 write(1, "11111111111\n11111111111111111111"..., 4096) = 4096 read(0, "1111111111111111111\n111111111111"..., 4096) = 4096 write(1, "111111111111111\n1111111111111111"..., 4096) = 4096 read(0, "11111111111111111111111\n11111111"..., 4096) = 4096 write(1, "1111111111111111111\n111111111111"..., 4096) = 4096 read(0, "11\n111111111111111111111111\n1111"..., 4096) = 4096 write(1, "11111111111111111111111\n11111111"..., 4096) = 4096 read(0, "111111\n111111111111111111111111\n"..., 4096) = 4096 write(1, "11\n111111111111111111111111\n1111"..., 4096) = 4096 read(0, "1111111111\n111111111111111111111"..., 4096) = 4096 write(1, "111111\n111111111111111111111111\n"..., 4096) = 4096 read(0, "11111111111111\n11111111111111111"..., 4096) = 4096 write(1, "1111111111\n111111111111111111111"..., 4096) = 4096 read(0, "111111111111111111\n1111111111111"..., 4096) = 4096 write(1, "11111111111111\n11111111111111111"..., 4096) = 4096 read(0, "1111111111111111111111\n111111111"..., 4096) = 4096 write(1, "111111111111111111\n1111111111111"..., 4096) = 4096 read(0, "1\n111111111111111111111111\n11111"..., 4096) = 4096 write(1, "1111111111111111111111\n111111111"..., 4096) = 4096 read(0, "11111\n111111111111111111111111\n1"..., 4096) = 4096 write(1, "1\n111111111111111111111111\n11111"..., 4096) = 4096 read(0, "111111111\n1111111111111111111111"..., 4096) = 4096 write(1, "11111\n111111111111111111111111\n1"..., 4096) = 4096 read(0, "1111111111111\n111111111111111111"..., 4096) = 4096 write(1, "111111111\n1111111111111111111111"..., 4096) = 4096 read(0, "11111111111111111\n11111111111111"..., 4096) = 4096 write(1, "1111111111111\n111111111111111111"..., 4096) = -1 EPIPE (Broken pipe) --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=5886, si_uid=0} --- +++ killed by SIGPIPE +++ Try grep, cut, fold, and dozens of other tools. They are all buggy. ;)
(In reply to comment #9) > Try grep, cut, fold, and dozens of other tools. They are all buggy. ;) Seems that you have missed my point. In all your examples process get killed by SIGPIPE with *default* signal handler. However on Android system (at least on Android 5.0 and later) dynamic linker changes signal handler for SIGPIPE to 'debugger_signal_handler' function (i suggest to read this article for more information: https://kobablog.wordpress.com/2011/05/12/debuggerd-of-android/). So if ps dies on Android it will not be killed by SIGPIPE, it will report to debuggerd.
(In reply to comment #10) > (In reply to comment #9) > > > Try grep, cut, fold, and dozens of other tools. They are all buggy. ;) > > Seems that you have missed my point. In all your examples process get killed by > SIGPIPE with *default* signal handler. However on Android system (at least on > Android 5.0 and later) dynamic linker changes signal handler for SIGPIPE to > 'debugger_signal_handler' function. I have two arguments against it: (1) I am not aware of any standards which allow libc init code or dynamic linker to install a non-default SIGPIPE handler. (2) Please tell me where do you see "cat" resetting SIGPIPE handler to default here - $ strace cat </dev/zero | sleep 1 execve("/usr/bin/cat", ["cat"], [/* 52 vars */]) = 0 brk(0) = 0x61e000 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f1fa65f1000 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) open("/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3 fstat(3, {st_mode=S_IFREG|0644, st_size=91352, ...}) = 0 mmap(NULL, 91352, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f1fa65da000 close(3) = 0 open("/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\200\34\2\0\0\0\0\0"..., 832) = 832 fstat(3, {st_mode=S_IFREG|0755, st_size=2101016, ...}) = 0 mmap(NULL, 3928640, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f1fa6012000 mprotect(0x7f1fa61c7000, 2097152, PROT_NONE) = 0 mmap(0x7f1fa63c7000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1b5000) = 0x7f1fa63c7000 mmap(0x7f1fa63cd000, 16960, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f1fa63cd000 close(3) = 0 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f1fa65d9000 mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f1fa65d7000 arch_prctl(ARCH_SET_FS, 0x7f1fa65d7740) = 0 mprotect(0x7f1fa63c7000, 16384, PROT_READ) = 0 mprotect(0x60a000, 4096, PROT_READ) = 0 mprotect(0x7f1fa65f2000, 4096, PROT_READ) = 0 munmap(0x7f1fa65da000, 91352) = 0 brk(0) = 0x61e000 brk(0x63f000) = 0x63f000 brk(0) = 0x63f000 open("/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = 3 fstat(3, {st_mode=S_IFREG|0644, st_size=104789808, ...}) = 0 mmap(NULL, 104789808, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f1f9fc22000 close(3) = 0 fstat(1, {st_mode=S_IFIFO|0600, st_size=0, ...}) = 0 fstat(0, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 5), ...}) = 0 fadvise64(0, 0, 0, POSIX_FADV_SEQUENTIAL) = 0 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = 65536 write(1, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 65536) = -1 EPIPE (Broken pipe) --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=8155, si_uid=0} --- +++ killed by SIGPIPE +++ Do you see a sigaction syscall anywhere in the listing? I don't. It means that coreutils cat does not do anything like "I need to set SIGPIPE to SIG_DFL".
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > Do you see a sigaction syscall anywhere in the listing? > I don't. > It means that coreutils cat does not do anything like "I need to set SIGPIPE to > SIG_DFL". Yes, i agree with you, GNU coreutils indeed doesn't change signal disposition, howerver GNU coreutils relies on behavior of GNU dynamic linker (/lib/ld-linux.so). Apparently Android dynamic linker behaves differently from GNU loader. How do you think, is it good idea to ignore Android specific features?