cmdserver: add option to not exit from message loop on SIGINT
Sending SIGINT to server is the only way to interrupt a command running in
command-server process. SIGINT will be caught at dispatch.dispatch() if
we're lucky. Otherwise it will terminate the serer process. This is
fundamentally unreliable as signals are delivered asynchronously.
"cmdserver.shutdown-on-interrupt=False" mitigate the issue by making the
server basically block SIGINT.
--- a/mercurial/commandserver.py Mon Jul 06 17:51:18 2020 +0200
+++ b/mercurial/commandserver.py Sat Jun 27 21:46:23 2020 +0900
@@ -244,8 +244,23 @@
self.client = fin
+ # If shutdown-on-interrupt is off, the default SIGINT handler is
+ # removed so that client-server communication wouldn't be interrupted.
+ # For example, 'runcommand' handler will issue three short read()s.
+ # If one of the first two read()s were interrupted, the communication
+ # channel would be left at dirty state and the subsequent request
+ # wouldn't be parsed. So catching KeyboardInterrupt isn't enough.
+ self._shutdown_on_interrupt = ui.configbool(
+ b'cmdserver', b'shutdown-on-interrupt'
+ )
+ self._old_inthandler = None
+ if not self._shutdown_on_interrupt:
+ self._old_inthandler = signal.signal(signal.SIGINT, signal.SIG_IGN)
+
def cleanup(self):
"""release and restore resources taken during server session"""
+ if not self._shutdown_on_interrupt:
+ signal.signal(signal.SIGINT, self._old_inthandler)
def _read(self, size):
if not size:
@@ -278,6 +293,32 @@
else:
return []
+ def _dispatchcommand(self, req):
+ from . import dispatch # avoid cycle
+
+ if self._shutdown_on_interrupt:
+ # no need to restore SIGINT handler as it is unmodified.
+ return dispatch.dispatch(req)
+
+ try:
+ signal.signal(signal.SIGINT, self._old_inthandler)
+ return dispatch.dispatch(req)
+ except error.SignalInterrupt:
+ # propagate SIGBREAK, SIGHUP, or SIGTERM.
+ raise
+ except KeyboardInterrupt:
+ # SIGINT may be received out of the try-except block of dispatch(),
+ # so catch it as last ditch. Another KeyboardInterrupt may be
+ # raised while handling exceptions here, but there's no way to
+ # avoid that except for doing everything in C.
+ pass
+ finally:
+ signal.signal(signal.SIGINT, signal.SIG_IGN)
+ # On KeyboardInterrupt, print error message and exit *after* SIGINT
+ # handler removed.
+ req.ui.error(_(b'interrupted!\n'))
+ return -1
+
def runcommand(self):
""" reads a list of \0 terminated arguments, executes
and writes the return code to the result channel """
@@ -318,7 +359,10 @@
)
try:
- ret = dispatch.dispatch(req) & 255
+ ret = self._dispatchcommand(req) & 255
+ # If shutdown-on-interrupt is off, it's important to write the
+ # result code *after* SIGINT handler removed. If the result code
+ # were lost, the client wouldn't be able to continue processing.
self.cresult.write(struct.pack(b'>i', int(ret)))
finally:
# restore old cwd
--- a/mercurial/configitems.py Mon Jul 06 17:51:18 2020 +0200
+++ b/mercurial/configitems.py Sat Jun 27 21:46:23 2020 +0900
@@ -212,6 +212,9 @@
default=lambda: [b'chgserver', b'cmdserver', b'repocache'],
)
coreconfigitem(
+ b'cmdserver', b'shutdown-on-interrupt', default=True,
+)
+coreconfigitem(
b'color', b'.*', default=None, generic=True,
)
coreconfigitem(
--- a/mercurial/helptext/config.txt Mon Jul 06 17:51:18 2020 +0200
+++ b/mercurial/helptext/config.txt Sat Jun 27 21:46:23 2020 +0900
@@ -408,6 +408,18 @@
If no suitable authentication entry is found, the user is prompted
for credentials as usual if required by the remote.
+``cmdserver``
+-------------
+
+Controls command server settings. (ADVANCED)
+
+``shutdown-on-interrupt``
+ If set to false, the server's main loop will continue running after
+ SIGINT received. ``runcommand`` requests can still be interrupted by
+ SIGINT. Close the write end of the pipe to shut down the server
+ process gracefully.
+ (default: True)
+
``color``
---------
--- a/tests/test-commandserver.t Mon Jul 06 17:51:18 2020 +0200
+++ b/tests/test-commandserver.t Sat Jun 27 21:46:23 2020 +0900
@@ -734,6 +734,51 @@
$ cd ..
+#if no-windows
+
+option to not shutdown on SIGINT:
+
+ $ cat <<'EOF' > dbgint.py
+ > import os
+ > import signal
+ > import time
+ > from mercurial import commands, registrar
+ > cmdtable = {}
+ > command = registrar.command(cmdtable)
+ > @command(b"debugsleep", norepo=True)
+ > def debugsleep(ui):
+ > time.sleep(1)
+ > @command(b"debugsuicide", norepo=True)
+ > def debugsuicide(ui):
+ > os.kill(os.getpid(), signal.SIGINT)
+ > time.sleep(1)
+ > EOF
+
+ >>> import signal
+ >>> import time
+ >>> from hgclient import checkwith, readchannel, runcommand
+ >>> @checkwith(extraargs=[b'--config', b'cmdserver.shutdown-on-interrupt=False',
+ ... b'--config', b'extensions.dbgint=dbgint.py'])
+ ... def nointr(server):
+ ... readchannel(server)
+ ... server.send_signal(signal.SIGINT) # server won't be terminated
+ ... time.sleep(1)
+ ... runcommand(server, [b'debugsleep'])
+ ... server.send_signal(signal.SIGINT) # server won't be terminated
+ ... runcommand(server, [b'debugsleep'])
+ ... runcommand(server, [b'debugsuicide']) # command can be interrupted
+ ... server.send_signal(signal.SIGTERM) # server will be terminated
+ ... time.sleep(1)
+ *** runcommand debugsleep
+ *** runcommand debugsleep
+ *** runcommand debugsuicide
+ interrupted!
+ killed!
+ [255]
+
+#endif
+
+
structured message channel:
$ cat <<'EOF' >> repo2/.hg/hgrc