This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1736

osutil: add a function to unblock signals
ClosedPublic

Authored by quark on Dec 20 2017, 5:12 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG8652ab4046e4: osutil: add a function to unblock signals
Summary

Signals could be blocked by something like:

#include <unistd.h>
#include <signal.h>
int main(int argc, char * const argv[]) {
  sigset_t set;
  sigfillset(&set);
  sigprocmask(SIG_BLOCK, &set, NULL);
  execv("/bin/hg", argv);
  return 0;
}

One of the problems is if SIGCHLD is blocked, chgserver would not reap
zombie workers since it depends on SIGCHLD handler entirely.

While it's the parent process to blame but it seems a good idea to just
unblock the signal from hg. FWIW git does that for SIGPIPE already [1].

Unfortunately Python 2 does not reset or provide APIs to change signal
masks. Therefore let's add one in osutil. Note: Python 3.3 introduced
signal.pthread_sigmask which solves the problem.

sigprocmask is part of POSIX [2] so there is no feature testing in
setup.py.

[1]: https://github.com/git/git/commit/7559a1be8a0afb10df41d25e4cf4c5285a5faef1
[2]: http://pubs.opengroup.org/onlinepubs/7908799/xsh/sigprocmask.html

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Dec 20 2017, 5:12 AM
quark added a child revision: D1737: commandserver: unblock SIGCHLD.Dec 20 2017, 5:12 AM
quark edited the summary of this revision. (Show Details)
yuja requested changes to this revision.Dec 20 2017, 8:25 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/cext/osutil.c
1123

Nit: sigaddset() may raise EINVAL if the given signum is invalid.
Maybe it's also better to check the result of sigemptyset() for clarity.

This revision now requires changes to proceed.Dec 20 2017, 8:25 AM
quark updated this revision to Diff 4561.Dec 20 2017, 2:38 PM
yuja accepted this revision.Dec 21 2017, 9:34 AM
This revision is now accepted and ready to land.Dec 21 2017, 9:34 AM
Closed by commit rHG8652ab4046e4: osutil: add a function to unblock signals (authored by quark). · Explain WhyDec 21 2017, 9:54 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/osutil.c (24 lines)
Mmercurial/policy.py (2 lines)
Mmercurial/util.py (4 lines)

Diff 4569

mercurial/cext/osutil.c

#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#ifdef _WIN32 #ifdef _WIN32
#include <io.h> #include <io.h>
#include <windows.h> #include <windows.h>
#else #else
#include <dirent.h> #include <dirent.h>
#include <signal.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/types.h> #include <sys/types.h>
#include <unistd.h> #include <unistd.h>
#ifdef HAVE_LINUX_STATFS #ifdef HAVE_LINUX_STATFS
#include <linux/magic.h> #include <linux/magic.h>
#include <sys/vfs.h> #include <sys/vfs.h>
#endif #endif
memset(&buf, 0, sizeof(buf)); memset(&buf, 0, sizeof(buf));
r = statfs(path, &buf); r = statfs(path, &buf);
if (r != 0) if (r != 0)
return PyErr_SetFromErrno(PyExc_OSError); return PyErr_SetFromErrno(PyExc_OSError);
return Py_BuildValue("s", describefstype(&buf)); return Py_BuildValue("s", describefstype(&buf));
} }
#endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */ #endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */
static PyObject *unblocksignal(PyObject *self, PyObject *args)
{
int sig = 0;
int r;
if (!PyArg_ParseTuple(args, "i", &sig))
return NULL;
sigset_t set;
r = sigemptyset(&set);
if (r != 0)
yujaUnsubmitted
Not Done

Nit: sigaddset() may raise EINVAL if the given signum is invalid.
Maybe it's also better to check the result of sigemptyset() for clarity.

yuja: Nit: `sigaddset()` may raise EINVAL if the given signum is invalid. Maybe it's also better to…
return PyErr_SetFromErrno(PyExc_OSError);
r = sigaddset(&set, sig);
if (r != 0)
return PyErr_SetFromErrno(PyExc_OSError);
r = sigprocmask(SIG_UNBLOCK, &set, NULL);
if (r != 0)
return PyErr_SetFromErrno(PyExc_OSError);
Py_RETURN_NONE;
}
#endif /* ndef _WIN32 */ #endif /* ndef _WIN32 */
static PyObject *listdir(PyObject *self, PyObject *args, PyObject *kwargs) static PyObject *listdir(PyObject *self, PyObject *args, PyObject *kwargs)
{ {
PyObject *statobj = NULL; /* initialize - optional arg */ PyObject *statobj = NULL; /* initialize - optional arg */
PyObject *skipobj = NULL; /* initialize - optional arg */ PyObject *skipobj = NULL; /* initialize - optional arg */
char *path, *skip = NULL; char *path, *skip = NULL;
int wantstat, plen; int wantstat, plen;
#ifndef SETPROCNAME_USE_NONE #ifndef SETPROCNAME_USE_NONE
{"setprocname", (PyCFunction)setprocname, METH_VARARGS, {"setprocname", (PyCFunction)setprocname, METH_VARARGS,
"set process title (best-effort)\n"}, "set process title (best-effort)\n"},
#endif #endif
#if defined(HAVE_BSD_STATFS) || defined(HAVE_LINUX_STATFS) #if defined(HAVE_BSD_STATFS) || defined(HAVE_LINUX_STATFS)
{"getfstype", (PyCFunction)getfstype, METH_VARARGS, {"getfstype", (PyCFunction)getfstype, METH_VARARGS,
"get filesystem type (best-effort)\n"}, "get filesystem type (best-effort)\n"},
#endif #endif
{"unblocksignal", (PyCFunction)unblocksignal, METH_VARARGS,
"change signal mask to unblock a given signal\n"},
#endif /* ndef _WIN32 */ #endif /* ndef _WIN32 */
#ifdef __APPLE__ #ifdef __APPLE__
{ {
"isgui", (PyCFunction)isgui, METH_NOARGS, "isgui", (PyCFunction)isgui, METH_NOARGS,
"Is a CoreGraphics session available?" "Is a CoreGraphics session available?"
}, },
#endif #endif
{NULL, NULL} {NULL, NULL}
}; };
static const int version = 1; static const int version = 2;
#ifdef IS_PY3K #ifdef IS_PY3K
static struct PyModuleDef osutil_module = { static struct PyModuleDef osutil_module = {
PyModuleDef_HEAD_INIT, PyModuleDef_HEAD_INIT,
"osutil", "osutil",
osutil_doc, osutil_doc,
-1, -1,
methods methods

mercurial/policy.py

return fakelocals[modname] return fakelocals[modname]
# keep in sync with "version" in C modules # keep in sync with "version" in C modules
_cextversions = { _cextversions = {
(r'cext', r'base85'): 1, (r'cext', r'base85'): 1,
(r'cext', r'bdiff'): 1, (r'cext', r'bdiff'): 1,
(r'cext', r'diffhelpers'): 1, (r'cext', r'diffhelpers'): 1,
(r'cext', r'mpatch'): 1, (r'cext', r'mpatch'): 1,
(r'cext', r'osutil'): 1, (r'cext', r'osutil'): 2,
(r'cext', r'parsers'): 4, (r'cext', r'parsers'): 4,
} }
# map import request to other package or module # map import request to other package or module
_modredirects = { _modredirects = {
(r'cext', r'charencode'): (r'cext', r'parsers'), (r'cext', r'charencode'): (r'cext', r'parsers'),
(r'cffi', r'base85'): (r'pure', r'base85'), (r'cffi', r'base85'): (r'pure', r'base85'),
(r'cffi', r'charencode'): (r'pure', r'charencode'), (r'cffi', r'charencode'): (r'pure', r'charencode'),

mercurial/util.py

try: try:
recvfds = osutil.recvfds recvfds = osutil.recvfds
except AttributeError: except AttributeError:
pass pass
try: try:
setprocname = osutil.setprocname setprocname = osutil.setprocname
except AttributeError: except AttributeError:
pass pass
try:
unblocksignal = osutil.unblocksignal
except AttributeError:
pass
# Python compatibility # Python compatibility
_notset = object() _notset = object()
# disable Python's problematic floating point timestamps (issue4836) # disable Python's problematic floating point timestamps (issue4836)
# (Python hypocritically says you shouldn't change this behavior in # (Python hypocritically says you shouldn't change this behavior in
# libraries, and sure enough Mercurial is not a library.) # libraries, and sure enough Mercurial is not a library.)