Skip to content
Snippets Groups Projects
Commit 01657bfb authored by Alexander Turenko's avatar Alexander Turenko Committed by Kirill Yukhin
Browse files

popen: always free resources in popen_delete()

The function still set a diagnostics when a signal sending fails and
returns -1, but it is purely informational result: for logging or so. It
reflects notes about dealing with failures in Linux's `man 2 close`:

 | Note, however, that a failure return should be used only for
 | diagnostic purposes <...> or remedial purposes <...>.
 |
 | <...> Linux  kernel always releases the file descriptor early in the
 | close operation, freeing it for reuse; the steps that may return an
 | error <...> occur only later in the close operation.
 |
 | Many other implementations similarly always close the file descriptor
 | <...> even if they subsequently report an error on return from
 | close(). POSIX.1 is currently silent on this point, but there are
 | plans to mandate this behavior in the next major release of the
 | standard.

When kill or killpg returns EPERM a caller usually unable to overcome it
somehow: retrying is not sufficient here. So there are no needs to keep
the handle: a caller refuses the handle and don't want to perform any
other operation on it.  The open engine do its best to kill a child
process or a process group, but when it is not possible, just set the a
diagnostic and free handle resources anyway.

Left comments about observed Mac OS behaviour regarding killing a
process group, where all processes are zombies (or just when a process
group leader is zombie, don't sure): it gives EPERM instead of ESRCH
from killpg(). This result should not surprise a user, so it should be
documented. See [1] for another description of the problem (I don't find
any official information about this).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1329528



Part of #4031

Acked-by: default avatarCyrill Gorcunov <gorcunov@gmail.com>
(cherry picked from commit 56a8c346ecb0581300a63c6e677d8a4672ff1f95)
parent 06edcbe1
No related branches found
No related tags found
No related merge requests found
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment