Patchwork D1736: osutil: add a function to unblock signals

login
register
mail settings
Submitter phabricator
Date Dec. 20, 2017, 10:12 a.m.
Message ID <differential-rev-PHID-DREV-tn7amxq6nti2d4a634kb-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26368/
State Superseded
Headers show

Comments

phabricator - Dec. 20, 2017, 10:12 a.m.
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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].
  
  Unfortunate but 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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1736

AFFECTED FILES
  mercurial/cext/osutil.c
  mercurial/policy.py

CHANGE DETAILS




To: quark, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 20, 2017, 1:25 p.m.
yuja requested changes to this revision.
yuja added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> osutil.c:1123
> +	sigemptyset(&set);
> +	sigaddset(&set, sig);
> +	r = sigprocmask(SIG_UNBLOCK, &set, NULL);

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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1736

To: quark, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/policy.py b/mercurial/policy.py
--- a/mercurial/policy.py
+++ b/mercurial/policy.py
@@ -74,7 +74,7 @@ 
     (r'cext', r'bdiff'): 1,
     (r'cext', r'diffhelpers'): 1,
     (r'cext', r'mpatch'): 1,
-    (r'cext', r'osutil'): 1,
+    (r'cext', r'osutil'): 2,
     (r'cext', r'parsers'): 4,
 }
 
diff --git a/mercurial/cext/osutil.c b/mercurial/cext/osutil.c
--- a/mercurial/cext/osutil.c
+++ b/mercurial/cext/osutil.c
@@ -20,6 +20,7 @@ 
 #include <windows.h>
 #else
 #include <dirent.h>
+#include <signal.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
@@ -1111,6 +1112,21 @@ 
 }
 #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;
+	sigemptyset(&set);
+	sigaddset(&set, sig);
+	r = sigprocmask(SIG_UNBLOCK, &set, NULL);
+	if (r != 0)
+		return PyErr_SetFromErrno(PyExc_OSError);
+	Py_RETURN_NONE;
+}
+
 #endif /* ndef _WIN32 */
 
 static PyObject *listdir(PyObject *self, PyObject *args, PyObject *kwargs)
@@ -1291,6 +1307,8 @@ 
 	{"getfstype", (PyCFunction)getfstype, METH_VARARGS,
 	 "get filesystem type (best-effort)\n"},
 #endif
+	{"unblocksignal", (PyCFunction)unblocksignal, METH_VARARGS,
+	 "change signal mask to unblock a given signal\n"},
 #endif /* ndef _WIN32 */
 #ifdef __APPLE__
 	{
@@ -1301,7 +1319,7 @@ 
 	{NULL, NULL}
 };
 
-static const int version = 1;
+static const int version = 2;
 
 #ifdef IS_PY3K
 static struct PyModuleDef osutil_module = {