Use QSocketNotifier for userscripts.

qutebrowser/commands/userscripts.py

@@ -22,67 +22,44 @@
 import os
 import os.path
 import tempfile
-import select
 
-from PyQt5.QtCore import (pyqtSignal, QObject, QThread, QStandardPaths,
+from PyQt5.QtCore import (pyqtSignal, pyqtSlot, QObject, QStandardPaths,
-                          QProcessEnvironment, QProcess, QUrl)
+                          QSocketNotifier, QProcessEnvironment, QProcess, QUrl)
 
 from qutebrowser.utils import message, log, objreg, standarddir
 from qutebrowser.commands import runners, cmdexc
 
 
-class _BlockingFIFOReader(QObject):
+class _QtFIFOReader(QObject):
 
-    """A worker which reads commands from a FIFO endlessly.
+    """A FIFO reader based on a QSocketNotifier."""
-
-    This is intended to be run in a separate QThread. It reads from the given
-    FIFO even across EOF so an userscript can write to it multiple times.
-
-    It uses select() so it can timeout once per second, checking if termination
-    was requested.
-
-    Attributes:
-        _filepath: The filename of the FIFO to read.
-        fifo: The file object which is being read.
-
-    Signals:
-        got_line: Emitted when a new line arrived.
-        finished: Emitted when the read loop realized it should terminate and
-                  is about to do so.
-    """
 
     got_line = pyqtSignal(str)
-    finished = pyqtSignal()
 
     def __init__(self, filepath, parent=None):
         super().__init__(parent)
         self._filepath = filepath
-        self.fifo = None
+        # We open as R/W so we never get EOF and have to reopen the pipe.
+        # See http://www.outflux.net/blog/archives/2008/03/09/using-select-on-a-fifo/
+        # We also use os.open and os.fdopen rather than built-in open so we
+        # can add O_NONBLOCK.
+        fd = os.open(filepath, os.O_RDWR |
+                     os.O_NONBLOCK)  # pylint: disable=no-member
+        self.fifo = os.fdopen(fd, 'r')
+        self._notifier = QSocketNotifier(fd, QSocketNotifier.Read, self)
+        self._notifier.activated.connect(self.read_line)
 
-    def read(self):
+    @pyqtSlot()
-        """Blocking read loop which emits got_line when a new line arrived."""
+    def read_line(self):
-        try:
+        """(Try to) read a line from the fifo."""
-            # We open as R/W so we never get EOF and have to reopen the pipe.
+        self._notifier.setEnabled(False)
-            # See http://www.outflux.net/blog/archives/2008/03/09/using-select-on-a-fifo/
+        for line in self.fifo:
-            # We also use os.open and os.fdopen rather than built-in open so we
+            self.got_line.emit(line.rstrip('\r\n'))
-            # can add O_NONBLOCK.
+        self._notifier.setEnabled(True)
-            fd = os.open(self._filepath, os.O_RDWR |
+
-                         os.O_NONBLOCK)  # pylint: disable=no-member
+    def cleanup(self):
-            self.fifo = os.fdopen(fd, 'r')
+        """Clean up so the fifo can be closed."""
-        except OSError:
+        self._notifier.setEnabled(False)
-            log.procs.exception("Failed to read FIFO")
-            self.finished.emit()
-            return
-        while True:
-            log.procs.debug("thread loop")
-            ready_r, _ready_w, _ready_e = select.select([self.fifo], [], [], 1)
-            if ready_r:
-                log.procs.debug("reading data")
-                for line in self.fifo:
-                    self.got_line.emit(line.rstrip())
-            if QThread.currentThread().isInterruptionRequested():
-                self.finished.emit()
-                return
 
 
 class _BaseUserscriptRunner(QObject):
@@ -184,20 +161,17 @@ class _BaseUserscriptRunner(QObject):
 
 class _POSIXUserscriptRunner(_BaseUserscriptRunner):
 
-    """Userscript runner to be used on POSIX. Uses _BlockingFIFOReader.
+    """Userscript runner to be used on POSIX. Uses _QtFIFOReader.
 
-    The OS must have support for named pipes and select(). Commands are
+    Commands are executed immediately when they arrive in the FIFO.
-    executed immediately when they arrive in the FIFO.
 
     Attributes:
-        _reader: The _BlockingFIFOReader instance.
+        _reader: The _QtFIFOReader instance.
-        _thread: The QThread where reader runs.
     """
 
     def __init__(self, win_id, parent=None):
         super().__init__(win_id, parent)
         self._reader = None
-        self._thread = None
 
     def run(self, cmd, *args, env=None):
         rundir = standarddir.get(QStandardPaths.RuntimeLocation)
@@ -205,7 +179,7 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner):
             # tempfile.mktemp is deprecated and discouraged, but we use it here
             # to create a FIFO since the only other alternative would be to
             # create a directory and place the FIFO there, which sucks. Since
-            # os.kfifo will raise an exception anyways when the path doesn't
+            # os.mkfifo will raise an exception anyways when the path doesn't
             # exist, it shouldn't be a big issue.
             self._filepath = tempfile.mktemp(prefix='userscript-', dir=rundir)
             os.mkfifo(self._filepath)  # pylint: disable=no-member
@@ -214,41 +188,31 @@ class _POSIXUserscriptRunner(_BaseUserscriptRunner):
                 e))
             return
 
-        self._reader = _BlockingFIFOReader(self._filepath)
+        self._reader = _QtFIFOReader(self._filepath)
-        self._thread = QThread(self)
-        self._reader.moveToThread(self._thread)
         self._reader.got_line.connect(self.got_cmd)
-        self._thread.started.connect(self._reader.read)
-        self._reader.finished.connect(self.on_reader_finished)
-        self._thread.finished.connect(self.on_thread_finished)
 
         self._run_process(cmd, *args, env=env)
-        self._thread.start()
 
     def on_proc_finished(self):
         """Interrupt the reader when the process finished."""
         log.procs.debug("proc finished")
-        self._thread.requestInterruption()
+        self.finish()
 
     def on_proc_error(self, error):
         """Interrupt the reader when the process had an error."""
         super().on_proc_error(error)
-        self._thread.requestInterruption()
+        self.finish()
 
-    def on_reader_finished(self):
+    def finish(self):
         """Quit the thread and clean up when the reader finished."""
-        log.procs.debug("reader finished")
+        log.procs.debug("Cleaning up")
-        self._thread.quit()
+        self._reader.cleanup()
         self._reader.fifo.close()
         self._reader.deleteLater()
+        self._reader = None
         super()._cleanup()
         self.finished.emit()
 
-    def on_thread_finished(self):
-        """Clean up the QThread object when the thread finished."""
-        log.procs.debug("thread finished")
-        self._thread.deleteLater()
-
 
 class _WindowsUserscriptRunner(_BaseUserscriptRunner):