Timeout and die if LibreOffice hangs (#140)

Co-authored-by: Lennart Regebro <[email protected]>

CHANGES.rst

@@ -1,6 +1,9 @@
 2.3 (unreleased)
 ----------------
 
+- Added a --conversion-timeout argument to ``unoserver``, which causes unoserver
+  to fail if a conversion doesn't finish within a certain time.
+
 - By default it will now use the `soffice`` executable instead of `libreoffice`,
   as I had a problem with it using 100% load when started as `libreoffice`.
 

README.rst

@@ -86,7 +86,7 @@ Usage
 -----
 
 Installing unoserver installs three scripts, `unoserver`, `unoconverter` and `unocompare`.
-The server can also be run as a module with `python3 -m unoserver.server`, with the same 
+The server can also be run as a module with `python3 -m unoserver.server`, with the same
 arguments as the main script, which can be useful as it must be run with the LibreOffice
 provided Python.
 
@@ -109,6 +109,7 @@ Unoserver
 * `--user-installation`: The path to the LibreOffice user profile, defaults to a dynamically created temporary directory
 * `--libreoffice-pid-file`: If set, unoserver will write the Libreoffice PID to this file.
   If started in daemon mode, the file will not be deleted when unoserver exits.
+* `--conversion-timeout`: Terminate Libreoffice and exit if a conversion does not complete in the given time (in seconds).
 * `-v, --version`: Display version and exit.
 
 Unoconvert
@@ -167,10 +168,21 @@ Client/Server installations
 
 If you are installing Unoserver on a dedicated machine (virtual or not) to do the conversions and
 are running the commands from a different machine, or if you want to call the convert/compare commands
-from Python directly, the clients do not need access to Libreoffice. You can therefore follow the 
+from Python directly, the clients do not need access to Libreoffice. You can therefore follow the
 instructions above to make Unoserver have access to the LibreOffice library, but on the client
-side you can simply install Unoserver as any other Python library, with `python -m pip install unoserver` 
-using the Python you want to use as the client executable. 
+side you can simply install Unoserver as any other Python library, with `python -m pip install unoserver`
+using the Python you want to use as the client executable.
+
+Please note that there is no security on either ports used, and as a result Unoserver is vulnerable
+to DDOS attacks, and possibly worse. The ports used **must not** be accessible to anything outside the
+server stack being used.
+
+Unoserver is designed to be started by some service management software, such as Supervisor or similar,
+that will restart the service should it crash. Unoserver does not try to restart LibreOffice if it
+crashes, but should instead also stop in that sitution. The ``--conversion-timeout`` argument will
+teminate LibreOffice if it takes to long to convert a document, and that termination will also result
+in Unoserver quitting. Because of this service monitoring software should be set up to restart
+Unoserver when it exits.
 
 
 Development and Testing

src/unoserver/server.py

@@ -16,6 +16,8 @@
 from importlib import metadata
 from pathlib import Path
 
+from concurrent import futures
+
 from unoserver import converter, comparer
 
 __version__ = metadata.version("unoserver")
@@ -48,12 +50,14 @@ def __init__(
         uno_interface="127.0.0.1",
         uno_port="2002",
         user_installation=None,
+        conversion_timeout=None,
     ):
         self.interface = interface
         self.uno_interface = uno_interface
         self.port = port
         self.uno_port = uno_port
         self.user_installation = user_installation
+        self.conversion_timeout = conversion_timeout
         self.libreoffice_process = None
         self.xmlrcp_thread = None
         self.xmlrcp_server = None
@@ -161,17 +165,27 @@ def convert(
                 if indata is not None:
                     indata = indata.data
 
-                result = self.conv.convert(
-                    inpath,
-                    indata,
-                    outpath,
-                    convert_to,
-                    filtername,
-                    filter_options,
-                    update_index,
-                    infiltername,
-                )
-                return result
+                with futures.ThreadPoolExecutor() as executor:
+                    future = executor.submit(
+                        self.conv.convert,
+                        inpath,
+                        indata,
+                        outpath,
+                        convert_to,
+                        filtername,
+                        filter_options,
+                        update_index,
+                        infiltername,
+                    )
+                    try:
+                        return future.result(timeout=self.conversion_timeout)
+                    except futures.TimeoutError:
+                        logger.error(
+                            "Conversion timeout, terminating conversion and exiting."
+                        )
+                        self.conv.local_context.dispose()
+                        self.libreoffice_process.terminate()
+                        raise
 
             @server.register_function
             def compare(
@@ -187,10 +201,25 @@ def compare(
                 if newdata is not None:
                     newdata = newdata.data
 
-                result = self.comp.compare(
-                    oldpath, olddata, newpath, newdata, outpath, filetype
-                )
-                return result
+                with futures.ThreadPoolExecutor() as executor:
+                    future = executor.submit(
+                        self.comp.compare,
+                        oldpath,
+                        olddata,
+                        newpath,
+                        newdata,
+                        outpath,
+                        filetype,
+                    )
+                try:
+                    return future.result(timeout=self.conversion_timeout)
+                except futures.TimeoutError:
+                    logger.error(
+                        "Comparison timeout, terminating conversion and exiting."
+                    )
+                    self.conv.local_context.dispose()
+                    self.libreoffice_process.terminate()
+                    raise
 
             server.serve_forever()
 
@@ -267,6 +296,12 @@ def main():
         help="If set, unoserver will write the Libreoffice PID to this file. If started "
         "in daemon mode, the file will not be deleted when unoserver exits.",
     )
+    parser.add_argument(
+        "--conversion-timeout",
+        type=int,
+        help="Terminate Libreoffice and exit if a conversion does not complete in the "
+        "given time (in seconds).",
+    )
     args = parser.parse_args()
 
     if args.daemon:
@@ -290,6 +325,7 @@ def main():
             args.uno_interface,
             args.uno_port,
             user_installation,
+            args.conversion_timeout,
         )
 
         if args.executable is not None: