From 2ece161a4c3ede44539453c79835b6d4aadd1e4b Mon Sep 17 00:00:00 2001 From: Lennart Regebro Date: Wed, 9 Oct 2024 15:11:58 +0200 Subject: [PATCH] Timeout and die if LibreOffice hangs --- CHANGES.rst | 3 ++ README.rst | 20 ++++++++++--- src/unoserver/server.py | 66 +++++++++++++++++++++++++++++++---------- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 904a8b0..a46fd24 100644 --- a/CHANGES.rst +++ b/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`. diff --git a/README.rst b/README.rst index 7534680..715dafd 100644 --- a/README.rst +++ b/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 diff --git a/src/unoserver/server.py b/src/unoserver/server.py index d8ec7c6..abe94cd 100644 --- a/src/unoserver/server.py +++ b/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: