Patchwork D3716: ui: add an uninterruptable context manager that can block SIGINT

login
register
mail settings
Submitter phabricator
Date June 27, 2018, 3:47 p.m.
Message ID <1735740fc0ffc7bdd1268ecddbbd5db5@localhost.localdomain>
Download mbox | patch
Permalink /patch/32469/
State Not Applicable
Headers show

Comments

phabricator - June 27, 2018, 3:47 p.m.
durin42 updated this revision to Diff 9326.
durin42 edited the summary of this revision.
durin42 retitled this revision from "ui: add an unsafeoperation context manager that can block SIGINT" to "ui: add an uninterruptable context manager that can block SIGINT".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3716?vs=9033&id=9326

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

AFFECTED FILES
  mercurial/configitems.py
  mercurial/ui.py
  mercurial/utils/procutil.py
  tests/test-nointerrupt.t

CHANGE DETAILS




To: durin42, #hg-reviewers, indygreg
Cc: spectral, indygreg, yuja, martinvonz, mercurial-devel
Yuya Nishihara - June 28, 2018, 12:15 p.m.
> +    @contextlib.contextmanager
> +    def uninterruptable(self):
> +        """Mark an operation as unsafe.
> +
> +        Most operations on a repository are safe to interrupt, but a
> +        few are risky (for example repair.strip). This context manager
> +        lets you advise Mercurial that something risky is happening so
> +        that control-C etc can be blocked if desired.
> +        """
> +        enabled = self.configbool('experimental', 'nointerrupt')
> +        if (enabled and
> +            self.configbool('experimental', 'nointerrupt-interactiveonly')):
> +            enabled = self.interactive()
> +        if self._uninterruptible or not enabled:
> +            # if nointerrupt support is turned off, the process isn't
> +            # interactive, or we're already in an uninterruptable
> +            # block, do nothing.
> +            yield
> +            return
> +        def warn():
> +            self.warn(_("shutting down cleanly\n"))
> +            self.warn(
> +                _("press ^C again to terminate immediately (dangerous)\n"))

Missed `return True` ?

Other that that, the patch looks good to me.
phabricator - June 28, 2018, 12:16 p.m.
yuja added a comment.


  > +    @contextlib.contextmanager
  >  +    def uninterruptable(self):
  >  +        """Mark an operation as unsafe.
  >  +
  >  +        Most operations on a repository are safe to interrupt, but a
  >  +        few are risky (for example repair.strip). This context manager
  >  +        lets you advise Mercurial that something risky is happening so
  >  +        that control-C etc can be blocked if desired.
  >  +        """
  >  +        enabled = self.configbool('experimental', 'nointerrupt')
  >  +        if (enabled and
  >  +            self.configbool('experimental', 'nointerrupt-interactiveonly')):
  >  +            enabled = self.interactive()
  >  +        if self._uninterruptible or not enabled:
  >  +            # if nointerrupt support is turned off, the process isn't
  >  +            # interactive, or we're already in an uninterruptable
  >  +            # block, do nothing.
  >  +            yield
  >  +            return
  >  +        def warn():
  >  +            self.warn(_("shutting down cleanly\n"))
  >  +            self.warn(
  >  +                _("press ^C again to terminate immediately (dangerous)\n"))
  
  Missed `return True` ?
  
  Other that that, the patch looks good to me.

REPOSITORY
  rHG Mercurial

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

To: durin42, #hg-reviewers, indygreg
Cc: spectral, indygreg, yuja, martinvonz, mercurial-devel

Patch

diff --git a/tests/test-nointerrupt.t b/tests/test-nointerrupt.t
new file mode 100644
--- /dev/null
+++ b/tests/test-nointerrupt.t
@@ -0,0 +1,83 @@ 
+Dummy extension simulating unsafe long running command
+  $ cat > sleepext.py <<EOF
+  > import time
+  > import itertools
+  > 
+  > from mercurial import registrar
+  > from mercurial.i18n import _
+  > 
+  > cmdtable = {}
+  > command = registrar.command(cmdtable)
+  > 
+  > @command(b'sleep', [], _(b'TIME'), norepo=True)
+  > def sleep(ui, sleeptime=b"1", **opts):
+  >     with ui.uninterruptable():
+  >         for _i in itertools.repeat(None, int(sleeptime)):
+  >             time.sleep(1)
+  >         ui.warn(b"end of unsafe operation\n")
+  >     ui.warn(b"%s second(s) passed\n" % sleeptime)
+  > EOF
+
+Kludge to emulate timeout(1) which is not generally available.
+  $ cat > timeout.py <<EOF
+  > from __future__ import print_function
+  > import argparse
+  > import signal
+  > import subprocess
+  > import sys
+  > import time
+  > 
+  > ap = argparse.ArgumentParser()
+  > ap.add_argument('-s', nargs=1, default='SIGTERM')
+  > ap.add_argument('duration', nargs=1, type=int)
+  > ap.add_argument('argv', nargs='*')
+  > opts = ap.parse_args()
+  > try:
+  >     sig = int(opts.s[0])
+  > except ValueError:
+  >     sname = opts.s[0]
+  >     if not sname.startswith('SIG'):
+  >         sname = 'SIG' + sname
+  >     sig = getattr(signal, sname)
+  > proc = subprocess.Popen(opts.argv)
+  > time.sleep(opts.duration[0])
+  > proc.poll()
+  > if proc.returncode is None:
+  >     proc.send_signal(sig)
+  >     proc.wait()
+  >     sys.exit(124)
+  > EOF
+
+Set up repository
+  $ hg init repo
+  $ cd repo
+  $ cat >> $HGRCPATH << EOF
+  > [extensions]
+  > sleepext = ../sleepext.py
+  > EOF
+
+Test ctrl-c
+  $ python $TESTTMP/timeout.py -s INT 1 hg sleep 2
+  interrupted!
+  [124]
+
+  $ cat >> $HGRCPATH << EOF
+  > [experimental]
+  > nointerrupt = yes
+  > EOF
+
+  $ python $TESTTMP/timeout.py -s INT 1 hg sleep 2
+  interrupted!
+  [124]
+
+  $ cat >> $HGRCPATH << EOF
+  > [experimental]
+  > nointerrupt-interactiveonly = False
+  > EOF
+
+  $ python $TESTTMP/timeout.py -s INT 1 hg sleep 2
+  shutting down cleanly
+  press ^C again to terminate immediately (dangerous)
+  end of unsafe operation
+  interrupted!
+  [124]
diff --git a/mercurial/utils/procutil.py b/mercurial/utils/procutil.py
--- a/mercurial/utils/procutil.py
+++ b/mercurial/utils/procutil.py
@@ -408,3 +408,36 @@ 
     finally:
         if prevhandler is not None:
             signal.signal(signal.SIGCHLD, prevhandler)
+
+@contextlib.contextmanager
+def uninterruptable(warn):
+    """Inhibit SIGINT handling on a region of code.
+
+    Note that if this is called in a non-main thread, it turns into a no-op.
+
+    Args:
+      warn: A callable which takes no arguments, and returns True if the
+            previous signal handling should be restored.
+    """
+
+    oldsiginthandler = [signal.getsignal(signal.SIGINT)]
+    shouldbail = []
+
+    def disabledsiginthandler(*args):
+        if warn():
+            signal.signal(signal.SIGINT, oldsiginthandler[0])
+            del oldsiginthandler[0]
+        shouldbail.append(True)
+
+    try:
+        try:
+            signal.signal(signal.SIGINT, disabledsiginthandler)
+        except ValueError:
+            # wrong thread, oh well, we tried
+            del oldsiginthandler[0]
+        yield
+    finally:
+        if oldsiginthandler:
+            signal.signal(signal.SIGINT, oldsiginthandler[0])
+        if shouldbail:
+            raise KeyboardInterrupt
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -224,6 +224,7 @@ 
         self._colormode = None
         self._terminfoparams = {}
         self._styles = {}
+        self._uninterruptible = False
 
         if src:
             self.fout = src.fout
@@ -334,6 +335,37 @@ 
             self._blockedtimes[key + '_blocked'] += \
                 (util.timer() - starttime) * 1000
 
+    @contextlib.contextmanager
+    def uninterruptable(self):
+        """Mark an operation as unsafe.
+
+        Most operations on a repository are safe to interrupt, but a
+        few are risky (for example repair.strip). This context manager
+        lets you advise Mercurial that something risky is happening so
+        that control-C etc can be blocked if desired.
+        """
+        enabled = self.configbool('experimental', 'nointerrupt')
+        if (enabled and
+            self.configbool('experimental', 'nointerrupt-interactiveonly')):
+            enabled = self.interactive()
+        if self._uninterruptible or not enabled:
+            # if nointerrupt support is turned off, the process isn't
+            # interactive, or we're already in an uninterruptable
+            # block, do nothing.
+            yield
+            return
+        def warn():
+            self.warn(_("shutting down cleanly\n"))
+            self.warn(
+                _("press ^C again to terminate immediately (dangerous)\n"))
+        with procutil.uninterruptable(warn):
+            try:
+                self._uninterruptible = True
+                yield
+            finally:
+                self._uninterruptible = False
+
+
     def formatter(self, topic, opts):
         return formatter.formatter(self, self, topic, opts)
 
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -560,6 +560,9 @@ 
 coreconfigitem('experimental', 'mergedriver',
     default=None,
 )
+coreconfigitem('experimental', 'nointerrupt', default=False)
+coreconfigitem('experimental', 'nointerrupt-interactiveonly', default=True)
+
 coreconfigitem('experimental', 'obsmarkers-exchange-debug',
     default=False,
 )