Patchwork chgserver: create new process group after fork (issue5051)

login
register
mail settings
Submitter Jun Wu
Date Jan. 19, 2016, 6:24 p.m.
Message ID <2f7c73708d285349652b.1453227847@x1c>
Download mbox | patch
Permalink /patch/12834/
State Superseded
Headers show

Comments

Jun Wu - Jan. 19, 2016, 6:24 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1453227613 0
#      Tue Jan 19 18:20:13 2016 +0000
# Node ID 2f7c73708d285349652b72ac774727ef2f0ebc84
# Parent  d2c5ad3deccb5a504e2553652b66a4110db68afb
chgserver: create new process group after fork (issue5051)

This is to make SIGTSTP work. Before the patch, the server process group is
considered "orphaned" and will ignore SIGTSTP, SIGTTIN, SIGTTOU, according to
POSIX. See the comment above `will_become_orphaned_pgrp` in `kernel/exit.c`
from Linux 4.3 for details.

SIGTSTP is important if chgserver runs some ncurses commend like `commit -i`.
Ncurses has its own SIGTSTP handler which will do the following:

  1. Clean the screen
  2. Stop itself by resending SIGTSTP to itself
  3. Restore the screen

If SIGTSTP is ignored, step 2 will be a noop, which means the process cannot
be suspended properly.

In order to make things work, chg client needs to forward SIGTSTP and SIGCONT
to server as well.
Jun Wu - Jan. 20, 2016, 10:30 a.m.
Just found setpgrp is in Python std lib. Will update the patch.

On 01/19/2016 06:24 PM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1453227613 0
> #      Tue Jan 19 18:20:13 2016 +0000
> # Node ID 2f7c73708d285349652b72ac774727ef2f0ebc84
> # Parent  d2c5ad3deccb5a504e2553652b66a4110db68afb
> chgserver: create new process group after fork (issue5051)
>
> This is to make SIGTSTP work. Before the patch, the server process group is
> considered "orphaned" and will ignore SIGTSTP, SIGTTIN, SIGTTOU, according to
> POSIX. See the comment above `will_become_orphaned_pgrp` in `kernel/exit.c`
> from Linux 4.3 for details.
>
> SIGTSTP is important if chgserver runs some ncurses commend like `commit -i`.
> Ncurses has its own SIGTSTP handler which will do the following:
>
>    1. Clean the screen
>    2. Stop itself by resending SIGTSTP to itself
>    3. Restore the screen
>
> If SIGTSTP is ignored, step 2 will be a noop, which means the process cannot
> be suspended properly.
>
> In order to make things work, chg client needs to forward SIGTSTP and SIGCONT
> to server as well.
>
> diff --git a/hgext/chgserver.py b/hgext/chgserver.py
> --- a/hgext/chgserver.py
> +++ b/hgext/chgserver.py
> @@ -29,6 +29,7 @@
>   from __future__ import absolute_import
>
>   import SocketServer
> +import ctypes, ctypes.util
>   import errno
>   import os
>   import re
> @@ -353,7 +354,13 @@
>
>   # copied from mercurial/commandserver.py
>   class _requesthandler(SocketServer.StreamRequestHandler):
> +    _libc = ctypes.CDLL(ctypes.util.find_library('c'), use_errno=True)
> +
>       def handle(self):
> +        # use a different process group from the master process, making this
> +        # process pass kernel "is_current_pgrp_orphaned" check so signals like
> +        # SIGTSTP, SIGTTIN, SIGTTOU are not ignored.
> +        self._libc.setpgid(0, 0)
>           ui = self.server.ui
>           repo = self.server.repo
>           sv = chgcmdserver(ui, repo, self.rfile, self.wfile, self.connection)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/hgext/chgserver.py b/hgext/chgserver.py
--- a/hgext/chgserver.py
+++ b/hgext/chgserver.py
@@ -29,6 +29,7 @@ 
 from __future__ import absolute_import
 
 import SocketServer
+import ctypes, ctypes.util
 import errno
 import os
 import re
@@ -353,7 +354,13 @@ 
 
 # copied from mercurial/commandserver.py
 class _requesthandler(SocketServer.StreamRequestHandler):
+    _libc = ctypes.CDLL(ctypes.util.find_library('c'), use_errno=True)
+
     def handle(self):
+        # use a different process group from the master process, making this
+        # process pass kernel "is_current_pgrp_orphaned" check so signals like
+        # SIGTSTP, SIGTTIN, SIGTTOU are not ignored.
+        self._libc.setpgid(0, 0)
         ui = self.server.ui
         repo = self.server.repo
         sv = chgcmdserver(ui, repo, self.rfile, self.wfile, self.connection)