From cedd433c365a12c8b40f4de18041b5afb5f2a368 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 16 May 2019 17:52:36 +0300 Subject: [PATCH 01/20] improve logging --- .../handler/ooo/application/application.py | 40 ++++++++++++++----- .../handler/ooo/application/openoffice.py | 11 ++--- cloudooo/handler/ooo/handler.py | 12 +++++- cloudooo/handler/ooo/mimemapper.py | 5 ++- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 89145d7..8ccf8c8 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -29,8 +29,7 @@ from zope.interface import implements from cloudooo.interfaces.application import IApplication from cloudooo.util import logger -from cloudooo.handler.ooo.util import waitStopDaemon -from psutil import pid_exists, Process, AccessDenied +from psutil import pid_exists, Process, AccessDenied, TimeoutExpired, NoSuchProcess class Application(object): @@ -47,17 +46,36 @@ class Application(object): self.getAddress()[-1], self.pid())) - def stop(self): + def stopProcess(self, process_pid): """Stop the process""" - if hasattr(self, 'process') and self.status(): - process_pid = self.process.pid - logger.debug("Stop Pid - %s" % process_pid) + error = False + logger.debug("Stop Pid - %s", process_pid) + returncode = None + try: + process = Process(process_pid) + cmdline = " ".join(process.cmdline()) + process.terminate() + returncode = process.wait(self.timeout) + except NoSuchProcess: + pass + except TimeoutExpired: + error = True + logger.error("Process %s survived SIGTERM after %s", process_pid, self.timeout) try: - self.process.terminate() - waitStopDaemon(self, self.timeout) - finally: - if pid_exists(process_pid) or self.status(): - Process(process_pid).kill() + process.kill() + returncode = process.wait(self.timeout) + except NoSuchProcess: + pass + except TimeoutExpired: + logger.error("Process %s survived SIGKILL after %s", process_pid, self.timeout) + if error and returncode: + logger.error("Process %s cmdline: %s ended with returncode %s", process_pid, cmdline, returncode) + elif returncode != 0: + logger.debug("Process %s ended with returncode %s", process_pid, returncode) + + def stop(self): + if hasattr(self, 'process'): + self.stopProcess(self.process.pid) delattr(self, "process") def loadSettings(self, hostname, port, path_run_dir, **kwargs): diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 4acee1b..6c57613 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -37,7 +37,7 @@ from application import Application from cloudooo.interfaces.lockable import ILockable from cloudooo.util import logger, convertStringToBool from cloudooo.handler.ooo.util import waitStartDaemon, \ - removeDirectory, waitStopDaemon, \ + removeDirectory, \ socketStatus @@ -105,7 +105,6 @@ class OpenOffice(Application): """Start OpenOffice.org process""" for i in range(5): self.stop() - waitStopDaemon(self, self.timeout) self.process = Popen(command, close_fds=True, env=env) @@ -121,14 +120,16 @@ class OpenOffice(Application): for connection in process.connections(): if connection.status == "LISTEN" and \ connection.laddr[1] == self.port: - process.terminate() + self.stopProcess(process.pid) + break except AccessDenied, e: pass except TypeError, e: # exception to prevent one psutil issue with zombie processes - logger.debug(e) + logger.error(e) except NotImplementedError, e: - logger.error("lsof isn't installed on this machine: " + str(e)) + logger.error("lsof isn't installed on this machine: %s", str(e)) + break def start(self, init=True): """Start Instance.""" diff --git a/cloudooo/handler/ooo/handler.py b/cloudooo/handler/ooo/handler.py index 9f2a6f8..c53ef36 100644 --- a/cloudooo/handler/ooo/handler.py +++ b/cloudooo/handler/ooo/handler.py @@ -119,11 +119,18 @@ class Handler(object): self._startTimeout() process = Popen(command_list, stdout=PIPE, stderr=PIPE, close_fds=True, env=openoffice.environment_dict.copy()) + logger.debug("Process %s running", process.pid) stdout, stderr = process.communicate() finally: self._stopTimeout() if pid_exists(process.pid): + logger.debug("Process %s terminated", process.pid) process.terminate() + if (process.returncode < 0 and process.returncode != -6) or stderr: + logger.error("Process %s command:%s", process.pid, " ".join(command_list)) + logger.error("Process %s stdout:%s", process.pid, stdout) + logger.error("Process %s stderr:%s", process.pid, stderr) + logger.debug("Process %s terminated with returncode %s", process.pid, process.returncode) return stdout, stderr def _callUnoConverter(self, *feature_list, **kw): @@ -131,7 +138,9 @@ class Handler(object): if not openoffice.status(): openoffice.start() command_list = self._getCommand(*feature_list, **kw) + logger.debug("run convert first") stdout, stderr = self._subprocess(command_list) + logger.debug("stop convert first") if not stdout and stderr: first_error = stderr logger.error(stderr) @@ -139,10 +148,11 @@ class Handler(object): openoffice.restart() kw['document_url'] = self.document.getUrl() command = self._getCommand(*feature_list, **kw) + logger.debug("run convert second") stdout, stderr = self._subprocess(command) + logger.debug("stop convert second") if not stdout and stderr: second_error = "\nerror of the second run: " + stderr - logger.error(second_error) raise Exception(first_error + second_error) return stdout, stderr diff --git a/cloudooo/handler/ooo/mimemapper.py b/cloudooo/handler/ooo/mimemapper.py index 187e9f6..27ea19c 100644 --- a/cloudooo/handler/ooo/mimemapper.py +++ b/cloudooo/handler/ooo/mimemapper.py @@ -33,6 +33,7 @@ from subprocess import STDOUT from zope.interface import implements from filter import Filter from os import environ, path +from cloudooo.util import logger from cloudooo.interfaces.mimemapper import IMimemapper from types import InstanceType import json @@ -124,8 +125,10 @@ class MimeMapper(object): "--hostname=%s" % hostname, "--port=%s" % port] - process = Popen(command, stdout=PIPE, stderr=STDOUT, close_fds=True) + process = Popen(command, stdout=PIPE, stderr=PIPE, close_fds=True) stdout, stderr = process.communicate() + if stderr: + logger.error(stderr) if process.returncode: raise ValueError(stdout) filter_dict, type_dict = json.loads(stdout) -- 2.30.9 From d1a52dd52a82804cfd728ec7d974affe27151d0f Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Wed, 22 May 2019 18:26:06 +0300 Subject: [PATCH 02/20] fix incomplete refactoring in 115e6f1bd7ea3854fddcc97dec48a9c8c2342483 --- cloudooo/handler/ooo/helper/openoffice_tester.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index 188dfa6..bafc18e 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -4,9 +4,9 @@ import helper_util from getopt import getopt, GetoptError -def test_openoffice(hostname, port): +def test_openoffice(hostname, port, uno_path, office_binary_path): try: - helper_util.getServiceManager(hostname, port) + helper_util.getServiceManager(hostname, port, uno_path, office_binary_path) return True except Exception, err: print err -- 2.30.9 From 8f3d652657954c6a95dbea96edf4b94a95aeb975 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 16 May 2019 18:13:52 +0300 Subject: [PATCH 03/20] close file opened in uno, use only one service_manager by process --- cloudooo/handler/ooo/helper/unoconverter.py | 78 ++++++++++----------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/cloudooo/handler/ooo/helper/unoconverter.py b/cloudooo/handler/ooo/helper/unoconverter.py index d8ad040..2405cbe 100755 --- a/cloudooo/handler/ooo/helper/unoconverter.py +++ b/cloudooo/handler/ooo/helper/unoconverter.py @@ -27,6 +27,7 @@ # ############################################################################## +import os import sys import csv import codecs @@ -75,19 +76,23 @@ Options: class UnoConverter(object): """A module to easily work with OpenOffice.org.""" - def __init__(self, hostname, port, document_url, source_format, uno_path, - office_binary_path, refresh): + def __init__(self, service_manager, document_url, source_format, refresh): """ """ - self.hostname = hostname - self.port = port + self.service_manager = service_manager self.document_url = document_url self.document_dir_path = dirname(document_url) self.source_format = source_format self.refresh = refresh - self.uno_path = uno_path - self.office_binary_path = office_binary_path self._load() + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + if traceback: + sys.stderr.write(str(traceback)) + self.document_loaded.close(True) + def _createProperty(self, name, value): """Create property""" from com.sun.star.beans import PropertyValue @@ -171,9 +176,7 @@ class UnoConverter(object): refresh argument tells to uno environment to replace dynamic properties of document before conversion """ - service_manager = helper_util.getServiceManager(self.hostname, self.port, - self.uno_path, - self.office_binary_path) + service_manager = self.service_manager desktop = service_manager.createInstance("com.sun.star.frame.Desktop") uno_url = self.systemPathToFileUrl(self.document_url) uno_document = desktop.loadComponentFromURL( @@ -210,11 +213,8 @@ class UnoConverter(object): dir=self.document_dir_path) property_list = self._getPropertyToExport(output_format) - try: - self.document_loaded.storeToURL(self.systemPathToFileUrl(output_url), - tuple(property_list)) - finally: - self.document_loaded.dispose() + self.document_loaded.storeToURL(self.systemPathToFileUrl(output_url), + tuple(property_list)) return output_url def getMetadata(self): @@ -242,9 +242,7 @@ class UnoConverter(object): except AttributeError: pass - service_manager = helper_util.getServiceManager(self.hostname, self.port, - self.uno_path, - self.office_binary_path) + service_manager = self.service_manager type_detection = service_manager.createInstance("com.sun.star.document.TypeDetection") uno_file_access = service_manager.createInstance("com.sun.star.ucb.SimpleFileAccess") doc = uno_file_access.openFileRead(self.systemPathToFileUrl(self.document_url)) @@ -286,7 +284,6 @@ class UnoConverter(object): user_defined_properties.addProperty(prop, 0, '') user_defined_properties.setPropertyValue(prop, value) self.document_loaded.store() - self.document_loaded.dispose() def help(): @@ -317,7 +314,6 @@ def main(): import json except ImportError: import simplejson as json - refresh = None hostname = port = document_url = office_binary_path = uno_path =\ destination_format = source_format = refresh = metadata = mimemapper = None for opt, arg in iter(opt_list): @@ -345,28 +341,28 @@ def main(): elif opt == '--mimemapper': mimemapper = json.loads(arg) - - unoconverter = UnoConverter(hostname, port, document_url, source_format, - uno_path, office_binary_path, refresh) - if "--convert" in param_list and not '--getmetadata' in param_list \ - and not destination_format: - output = unoconverter.convert() - elif '--convert' in param_list and destination_format: - output = unoconverter.convert(destination_format) - elif '--getmetadata' in param_list and not '--convert' in param_list: - metadata_dict = unoconverter.getMetadata() - output = encodestring(json.dumps(metadata_dict).encode('utf-8')).decode('utf-8') - elif '--getmetadata' in param_list and '--convert' in param_list: - document_url = unoconverter.convert() - # Instanciate new UnoConverter instance with new url - unoconverter = UnoConverter(hostname, port, document_url, source_format, - uno_path, office_binary_path, refresh) - metadata_dict = unoconverter.getMetadata() - metadata_dict['document_url'] = document_url - output = encodestring(json.dumps(metadata_dict).encode('utf-8')).decode('utf-8') - elif '--setmetadata' in param_list: - unoconverter.setMetadata(metadata) - output = document_url + service_manager = helper_util.getServiceManager(hostname, port, + uno_path, office_binary_path) + if '--getmetadata' in param_list and '--convert' in param_list: + with UnoConverter(service_manager, document_url, source_format, refresh) as unoconverter: + document_url = unoconverter.convert() + # Instanciate new UnoConverter instance with new url + with UnoConverter(service_manager, document_url, source_format, refresh) as unoconverter: + metadata_dict = unoconverter.getMetadata() + metadata_dict['document_url'] = document_url + output = encodestring(json.dumps(metadata_dict).encode('utf-8')).decode('utf-8') + else: + with UnoConverter(service_manager, document_url, source_format, refresh) as unoconverter: + if "--convert" in param_list and not destination_format: + output = unoconverter.convert() + elif '--convert' in param_list and destination_format: + output = unoconverter.convert(destination_format) + elif '--getmetadata' in param_list: + metadata_dict = unoconverter.getMetadata() + output = encodestring(json.dumps(metadata_dict).encode('utf-8')).decode('utf-8') + elif '--setmetadata' in param_list: + unoconverter.setMetadata(metadata) + output = document_url sys.stdout.write(output) -- 2.30.9 From 5102981a26559a47bbb10d6265269455e1dd9d0d Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 16 May 2019 18:15:20 +0300 Subject: [PATCH 04/20] fix deadlock in uno thread --- cloudooo/handler/ooo/handler.py | 5 ++++- cloudooo/handler/ooo/helper/helper_util.py | 11 +++++++++++ .../handler/ooo/helper/openoffice_tester.py | 2 +- cloudooo/handler/ooo/helper/unoconverter.py | 3 +-- cloudooo/handler/ooo/helper/unomimemapper.py | 2 +- cloudooo/handler/ooo/mimemapper.py | 1 - .../handler/ooo/tests/testOooUnoConverter.py | 6 +++--- .../handler/ooo/tests/testOooUnoMimemapper.py | 18 +++++++++--------- 8 files changed, 30 insertions(+), 18 deletions(-) diff --git a/cloudooo/handler/ooo/handler.py b/cloudooo/handler/ooo/handler.py index c53ef36..79e4e69 100644 --- a/cloudooo/handler/ooo/handler.py +++ b/cloudooo/handler/ooo/handler.py @@ -126,11 +126,14 @@ class Handler(object): if pid_exists(process.pid): logger.debug("Process %s terminated", process.pid) process.terminate() - if (process.returncode < 0 and process.returncode != -6) or stderr: + if (process.returncode != -3) or stderr: logger.error("Process %s command:%s", process.pid, " ".join(command_list)) logger.error("Process %s stdout:%s", process.pid, stdout) logger.error("Process %s stderr:%s", process.pid, stderr) logger.debug("Process %s terminated with returncode %s", process.pid, process.returncode) + if process.returncode != -3: + raise ValueError("unocnverter.py should always be interrupted by external signal," + " otherwise deadlock can be raised") return stdout, stderr def _callUnoConverter(self, *feature_list, **kw): diff --git a/cloudooo/handler/ooo/helper/helper_util.py b/cloudooo/handler/ooo/helper/helper_util.py index 4d25282..90d480c 100644 --- a/cloudooo/handler/ooo/helper/helper_util.py +++ b/cloudooo/handler/ooo/helper/helper_util.py @@ -1,3 +1,4 @@ +import signal import sys import os import time @@ -31,3 +32,13 @@ def getServiceManager(host, port, uno_path, office_binary_path): time.sleep(1) # Get the ServiceManager object return uno_connection.ServiceManager + +def exitOverAbort(main_func): + try: + main_func() + except: + import traceback + sys.stderr.write(traceback.format_exc()) + sys.stdout.flush() + sys.stderr.flush() + os.kill(os.getpid(), signal.SIGQUIT) \ No newline at end of file diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index bafc18e..ea344ec 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -37,4 +37,4 @@ def main(): if __name__ == "__main__": - main() + helper_util.exitOverAbort(main) \ No newline at end of file diff --git a/cloudooo/handler/ooo/helper/unoconverter.py b/cloudooo/handler/ooo/helper/unoconverter.py index 2405cbe..784cbf9 100755 --- a/cloudooo/handler/ooo/helper/unoconverter.py +++ b/cloudooo/handler/ooo/helper/unoconverter.py @@ -27,7 +27,6 @@ # ############################################################################## -import os import sys import csv import codecs @@ -367,4 +366,4 @@ def main(): sys.stdout.write(output) if "__main__" == __name__: - main() + helper_util.exitOverAbort(main) \ No newline at end of file diff --git a/cloudooo/handler/ooo/helper/unomimemapper.py b/cloudooo/handler/ooo/helper/unomimemapper.py index 7ee306d..a2f3ee7 100755 --- a/cloudooo/handler/ooo/helper/unomimemapper.py +++ b/cloudooo/handler/ooo/helper/unomimemapper.py @@ -141,4 +141,4 @@ def main(): sort_keys=True, indent=2, separators=(',', ':'))) if "__main__" == __name__: - main() + helper_util.exitOverAbort(main) \ No newline at end of file diff --git a/cloudooo/handler/ooo/mimemapper.py b/cloudooo/handler/ooo/mimemapper.py index 27ea19c..256869d 100644 --- a/cloudooo/handler/ooo/mimemapper.py +++ b/cloudooo/handler/ooo/mimemapper.py @@ -129,7 +129,6 @@ class MimeMapper(object): stdout, stderr = process.communicate() if stderr: logger.error(stderr) - if process.returncode: raise ValueError(stdout) filter_dict, type_dict = json.loads(stdout) diff --git a/cloudooo/handler/ooo/tests/testOooUnoConverter.py b/cloudooo/handler/ooo/tests/testOooUnoConverter.py index 85a01dc..6d415cf 100644 --- a/cloudooo/handler/ooo/tests/testOooUnoConverter.py +++ b/cloudooo/handler/ooo/tests/testOooUnoConverter.py @@ -75,9 +75,9 @@ class TestUnoConverter(HandlerTestCase): "--destination_format=%s" % "doc", "--source_format=%s" % "odt", "--mimemapper=%s" % mimemapper_pickled] - stdout, stderr = Popen(command, - stdout=PIPE, - stderr=PIPE).communicate() + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) self.assertEquals(stderr, '') output_url = stdout.replace('\n', '') self.assertTrue(exists(output_url), stdout) diff --git a/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py b/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py index 4c27d8e..87751e8 100644 --- a/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py +++ b/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py @@ -63,9 +63,9 @@ class TestUnoMimeMapper(HandlerTestCase): "--office_binary_path=%s" % self.office_binary_path, "--hostname=%s" % self.hostname, "--port=%s" % self.openoffice_port] - stdout, stderr = Popen(command, - stdout=PIPE, - stderr=PIPE).communicate() + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -15) self.assertEquals(stderr, '') filter_dict, type_dict = json.loads(stdout) self.assertTrue('filter_dict' in locals()) @@ -84,9 +84,9 @@ class TestUnoMimeMapper(HandlerTestCase): "/helper/unomimemapper.py"), "--hostname=%s" % self.hostname, "--port=%s" % self.openoffice_port] - stdout, stderr = Popen(command, - stdout=PIPE, - stderr=PIPE).communicate() + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) self.assertEquals(stderr, '') filter_dict, type_dict = json.loads(stdout) self.assertTrue('filter_dict' in locals()) @@ -110,9 +110,9 @@ class TestUnoMimeMapper(HandlerTestCase): "--office_binary_path=%s" % self.office_binary_path, "--hostname=%s" % self.hostname, "--port=%s" % self.openoffice_port] - stdout, stderr = Popen(command, - stdout=PIPE, - stderr=PIPE).communicate() + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) self.assertEquals(stdout, '') self.assertTrue(stderr.endswith(error_msg), stderr) openoffice.start() -- 2.30.9 From 5edc710ca522de90f824db638444a5c07f477488 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Mon, 20 May 2019 20:37:17 +0300 Subject: [PATCH 05/20] cleanup --- cloudooo/handler/ooo/tests/testOooUnoConverter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudooo/handler/ooo/tests/testOooUnoConverter.py b/cloudooo/handler/ooo/tests/testOooUnoConverter.py index 6d415cf..adb8f71 100644 --- a/cloudooo/handler/ooo/tests/testOooUnoConverter.py +++ b/cloudooo/handler/ooo/tests/testOooUnoConverter.py @@ -31,7 +31,7 @@ import magic import pkg_resources from subprocess import Popen, PIPE from os.path import exists, join -from cloudooo.tests.handlerTestCase import HandlerTestCase, make_suite +from cloudooo.tests.handlerTestCase import HandlerTestCase from cloudooo.handler.ooo.application.openoffice import openoffice from cloudooo.handler.ooo.document import FileSystemDocument -- 2.30.9 From 6312f7e283ef49ade12766007b934f513583f795 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 23 May 2019 14:21:32 +0300 Subject: [PATCH 06/20] support python3 in openoffice_tester.py --- cloudooo/handler/ooo/helper/openoffice_tester.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index ea344ec..0f1279b 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -18,8 +18,8 @@ def main(): opt_list, arg_list = getopt(sys.argv[1:], "", ["port=", "hostname=", "uno_path=", "office_binary_path="]) - except GetoptError, e: - print >> sys.stderr, "%s \nUse --port and --hostname" % e + except GetoptError as e: + sys.stderr.write("%s \nUse --port and --hostname" % e) sys.exit(2) port = hostname = uno_path = office_binary_path = None @@ -33,7 +33,8 @@ def main(): elif opt == "--office_binary_path": office_binary_path = arg - print test_openoffice(hostname, port, uno_path, office_binary_path) + sys.stdout.write(str(test_openoffice(hostname, port, + uno_path, office_binary_path))) if __name__ == "__main__": -- 2.30.9 From bbe27cf2d1c13e8f40d049d994e543e4c9dfd455 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 23 May 2019 14:47:13 +0300 Subject: [PATCH 07/20] fix running openoffice_tester.py --- cloudooo/handler/ooo/application/openoffice.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 6c57613..f685b05 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -68,7 +68,8 @@ class OpenOffice(Application): "helper", "openoffice_tester.py")), "--hostname=%s" % host, "--port=%s" % port, - "--uno_path=%s" % self.uno_path] + "--uno_path=%s" % self.uno_path, + "--office_binary_path=%s" % self.office_binary_path] logger.debug("Testing Openoffice Instance %s" % port) stdout, stderr = Popen(args, stdout=PIPE, stderr=PIPE, close_fds=True).communicate() -- 2.30.9 From a073ec45dd1c089637282a5b5d3a8510314dc151 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 16 May 2019 18:09:51 +0300 Subject: [PATCH 08/20] rework @yusei patch a5157949 --- .../handler/ooo/application/openoffice.py | 21 ++++++++++++------- cloudooo/handler/ooo/helper/helper_util.py | 10 +-------- .../handler/ooo/helper/openoffice_tester.py | 17 ++++++++++----- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index f685b05..9aa0ab2 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -35,10 +35,14 @@ from threading import Lock from zope.interface import implements from application import Application from cloudooo.interfaces.lockable import ILockable -from cloudooo.util import logger, convertStringToBool +from cloudooo.util import logger from cloudooo.handler.ooo.util import waitStartDaemon, \ removeDirectory, \ socketStatus +try: + import json +except ImportError: + import simplejson as json class OpenOffice(Application): @@ -55,6 +59,7 @@ class OpenOffice(Application): """ self._bin_soffice = 'soffice.bin' self._lock = Lock() + self.last_test_error = None self._cleanRequest() def _testOpenOffice(self, host, port): @@ -70,16 +75,18 @@ class OpenOffice(Application): "--port=%s" % port, "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path] - logger.debug("Testing Openoffice Instance %s" % port) stdout, stderr = Popen(args, stdout=PIPE, stderr=PIPE, close_fds=True).communicate() - stdout_bool = convertStringToBool(stdout.replace("\n", "")) - if stdout_bool and stderr != "": - logger.debug("%s\n%s" % (stderr, stdout)) + if stdout == "": + logger.error("openoffice_tester.py cmdline: %s", " ".join(args)) + logger.error("openoffice_tester.py stdout: %s", stdout) + logger.error("openoffice_tester.py stderr: %s", stderr) return False - else: + stdout = json.loads(stdout) + self.last_test_error = stderr + if stdout == True: logger.debug("Instance %s works" % port) - return True + return stdout def _cleanRequest(self): """Define request attribute as 0""" diff --git a/cloudooo/handler/ooo/helper/helper_util.py b/cloudooo/handler/ooo/helper/helper_util.py index 90d480c..b674afb 100644 --- a/cloudooo/handler/ooo/helper/helper_util.py +++ b/cloudooo/handler/ooo/helper/helper_util.py @@ -1,7 +1,6 @@ import signal import sys import os -import time def getServiceManager(host, port, uno_path, office_binary_path): """Get the ServiceManager from the running OpenOffice.org. @@ -22,14 +21,7 @@ def getServiceManager(host, port, uno_path, office_binary_path): uno_context) # Connect to the running OpenOffice.org and get its # context. - # Retry 10 times if needed. - for i in range(10): - try: - uno_connection = resolver.resolve("uno:socket,host=%s,port=%s;urp;StarOffice.ComponentContext" % (host, port)) - break - except: - # I don't know how to import com.sun.star.connection.NoConnectException - time.sleep(1) + uno_connection = resolver.resolve("uno:socket,host=%s,port=%s,tcpNoDelay=1;urp;StarOffice.ComponentContext" % (host, port)) # Get the ServiceManager object return uno_connection.ServiceManager diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index 0f1279b..dfa4a10 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -3,13 +3,18 @@ import sys import helper_util from getopt import getopt, GetoptError +try: + import json +except ImportError: + import simplejson as json + def test_openoffice(hostname, port, uno_path, office_binary_path): + import pyuno try: helper_util.getServiceManager(hostname, port, uno_path, office_binary_path) return True - except Exception, err: - print err + except pyuno.getClass("com.sun.star.connection.NoConnectException"): return False @@ -30,12 +35,14 @@ def main(): hostname = arg elif opt == "--uno_path": uno_path = arg + if uno_path not in sys.path: + sys.path.append(uno_path) elif opt == "--office_binary_path": office_binary_path = arg - sys.stdout.write(str(test_openoffice(hostname, port, - uno_path, office_binary_path))) - + output = json.dumps(test_openoffice(hostname, port, + uno_path, office_binary_path)) + sys.stdout.write(output) if __name__ == "__main__": helper_util.exitOverAbort(main) \ No newline at end of file -- 2.30.9 From 467b6b869b7e36767353f0d2c31b88a178625c69 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 23 May 2019 01:32:43 +0300 Subject: [PATCH 09/20] use connection string as configure interlink between libreoffice and uno client --- .../handler/ooo/application/application.py | 14 +---- .../handler/ooo/application/openoffice.py | 60 ++++++++----------- cloudooo/handler/ooo/handler.py | 8 +-- cloudooo/handler/ooo/helper/helper_util.py | 4 +- .../handler/ooo/helper/openoffice_tester.py | 18 +++--- cloudooo/handler/ooo/helper/unoconverter.py | 12 ++-- cloudooo/handler/ooo/helper/unomimemapper.py | 16 +++-- cloudooo/handler/ooo/mimemapper.py | 5 +- .../handler/ooo/tests/testOooMimemapper.py | 4 +- .../handler/ooo/tests/testOooMonitorMemory.py | 1 - .../handler/ooo/tests/testOooUnoConverter.py | 5 +- .../handler/ooo/tests/testOooUnoMimemapper.py | 33 ++-------- cloudooo/handler/ooo/util.py | 15 ----- cloudooo/tests/handlerTestCase.py | 18 +++--- 14 files changed, 71 insertions(+), 142 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 8ccf8c8..0f04eb4 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -29,7 +29,7 @@ from zope.interface import implements from cloudooo.interfaces.application import IApplication from cloudooo.util import logger -from psutil import pid_exists, Process, AccessDenied, TimeoutExpired, NoSuchProcess +from psutil import pid_exists, Process, TimeoutExpired, NoSuchProcess class Application(object): @@ -100,17 +100,7 @@ class Application(object): pid = self.pid() if pid is None or not pid_exists(pid): return False - - process = Process(pid) - try: - for connection in process.connections(): - if connection.status == 'LISTEN' and \ - connection.laddr[1] == self.port: - return True - except AccessDenied: - return False - - return False + return True def getAddress(self): """Return port and hostname of OOo Instance.""" diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 9aa0ab2..34c126a 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -25,10 +25,8 @@ # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. # ############################################################################## - +import uuid import pkg_resources -import psutil -from psutil import AccessDenied from os.path import exists, join from subprocess import Popen, PIPE from threading import Lock @@ -37,12 +35,12 @@ from application import Application from cloudooo.interfaces.lockable import ILockable from cloudooo.util import logger from cloudooo.handler.ooo.util import waitStartDaemon, \ - removeDirectory, \ - socketStatus + removeDirectory try: import json except ImportError: import simplejson as json +from time import sleep class OpenOffice(Application): @@ -62,17 +60,16 @@ class OpenOffice(Application): self.last_test_error = None self._cleanRequest() - def _testOpenOffice(self, host, port): + def _testOpenOffice(self): """Test if OpenOffice was started correctly""" - logger.debug("Test OpenOffice %s - Pid %s" % (self.getAddress()[-1], + logger.debug("Test OpenOffice %s - Pid %s" % (self.getConnection(), self.pid())) python = join(self.office_binary_path, "python") args = [exists(python) and python or "python", pkg_resources.resource_filename("cloudooo", join('handler', 'ooo', "helper", "openoffice_tester.py")), - "--hostname=%s" % host, - "--port=%s" % port, + "--connection=%s" % self.getConnection(), "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path] stdout, stderr = Popen(args, stdout=PIPE, @@ -85,7 +82,7 @@ class OpenOffice(Application): stdout = json.loads(stdout) self.last_test_error = stderr if stdout == True: - logger.debug("Instance %s works" % port) + logger.debug("Instance %s works" % self.getConnection()) return stdout def _cleanRequest(self): @@ -108,6 +105,17 @@ class OpenOffice(Application): self.uno_path = uno_path self.default_language = default_language self.environment_dict = environment_dict + self.connection = "socket,host=%s,port=%d" % (self.hostname, self.port) + self.connection = "pipe,name=cloudooo_libreoffice_%s" % str(uuid.uuid1()) + + def status(self): + if Application.status(self) and \ + self._testOpenOffice(): + # repeat for check if uno client do not terminate + # libreoffice + if Application.status(self): + return True + return False def _startProcess(self, command, env): """Start OpenOffice.org process""" @@ -116,29 +124,15 @@ class OpenOffice(Application): self.process = Popen(command, close_fds=True, env=env) - if not waitStartDaemon(self, self.timeout): - continue - if self._testOpenOffice(self.hostname, self.port): - return - - def _releaseOpenOfficePort(self): - for process in psutil.process_iter(): - try: - if process.exe() == join(self.office_binary_path, self._bin_soffice): - for connection in process.connections(): - if connection.status == "LISTEN" and \ - connection.laddr[1] == self.port: - self.stopProcess(process.pid) - break - except AccessDenied, e: - pass - except TypeError, e: - # exception to prevent one psutil issue with zombie processes - logger.error(e) - except NotImplementedError, e: - logger.error("lsof isn't installed on this machine: %s", str(e)) + sleep(1) + if waitStartDaemon(self, self.timeout - 1): + if self.last_test_error: + logger.debug(self.last_test_error) break + def getConnection(self): + return self.connection + def start(self, init=True): """Start Instance.""" self.path_user_installation = join(self.path_run_dir, \ @@ -154,7 +148,7 @@ class OpenOffice(Application): '--nodefault', '--norestore', '--nofirststartwizard', - '--accept=socket,host=%s,port=%d;urp;' % (self.hostname, self.port), + '--accept=%s;urp;' % self.getConnection(), '-env:UserInstallation=file://%s' % self.path_user_installation, '--language=%s' % self.default_language, ] @@ -172,8 +166,6 @@ class OpenOffice(Application): """Stop the instance by pid. By the default the signal is 15.""" Application.stop(self) - if socketStatus(self.hostname, self.port): - self._releaseOpenOfficePort() self._cleanRequest() def isLocked(self): diff --git a/cloudooo/handler/ooo/handler.py b/cloudooo/handler/ooo/handler.py index 79e4e69..ae5f52f 100644 --- a/cloudooo/handler/ooo/handler.py +++ b/cloudooo/handler/ooo/handler.py @@ -83,13 +83,11 @@ class Handler(object): def _getCommand(self, *args, **kw): """Transforms all parameters passed in a command""" - hostname, port = openoffice.getAddress() - kw['hostname'] = hostname - kw['port'] = port python = path.join(self.office_binary_path, "python") command_list = [path.exists(python) and python or "python", pkg_resources.resource_filename(__name__, path.join("helper", "unoconverter.py")), + "--connection=%s" % openoffice.getConnection(), "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path, '--document_url=%s' % self.document.getUrl()] @@ -146,7 +144,6 @@ class Handler(object): logger.debug("stop convert first") if not stdout and stderr: first_error = stderr - logger.error(stderr) self.document.restoreOriginal() openoffice.restart() kw['document_url'] = self.document.getUrl() @@ -316,6 +313,5 @@ def bootstrapHandler(configuration_dict): # Load all filters openoffice.acquire() - mimemapper.loadFilterList(application_hostname, - openoffice_port, **kw) + mimemapper.loadFilterList(openoffice.getConnection(), **kw) openoffice.release() diff --git a/cloudooo/handler/ooo/helper/helper_util.py b/cloudooo/handler/ooo/helper/helper_util.py index b674afb..cb0d057 100644 --- a/cloudooo/handler/ooo/helper/helper_util.py +++ b/cloudooo/handler/ooo/helper/helper_util.py @@ -2,7 +2,7 @@ import signal import sys import os -def getServiceManager(host, port, uno_path, office_binary_path): +def getServiceManager(connection, uno_path, office_binary_path): """Get the ServiceManager from the running OpenOffice.org. """ # Add in sys.path the path of pyuno @@ -21,7 +21,7 @@ def getServiceManager(host, port, uno_path, office_binary_path): uno_context) # Connect to the running OpenOffice.org and get its # context. - uno_connection = resolver.resolve("uno:socket,host=%s,port=%s,tcpNoDelay=1;urp;StarOffice.ComponentContext" % (host, port)) + uno_connection = resolver.resolve("uno:%s;urp;StarOffice.ComponentContext" % connection) # Get the ServiceManager object return uno_connection.ServiceManager diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index dfa4a10..74887ff 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -9,10 +9,10 @@ except ImportError: import simplejson as json -def test_openoffice(hostname, port, uno_path, office_binary_path): +def test_openoffice(connection, uno_path, office_binary_path): import pyuno try: - helper_util.getServiceManager(hostname, port, uno_path, office_binary_path) + helper_util.getServiceManager(connection, uno_path, office_binary_path) return True except pyuno.getClass("com.sun.star.connection.NoConnectException"): return False @@ -21,18 +21,16 @@ def test_openoffice(hostname, port, uno_path, office_binary_path): def main(): try: opt_list, arg_list = getopt(sys.argv[1:], "", - ["port=", "hostname=", "uno_path=", + ["connection=", "uno_path=", "office_binary_path="]) except GetoptError as e: - sys.stderr.write("%s \nUse --port and --hostname" % e) + sys.stderr.write("%s \nUse --connection" % e) sys.exit(2) - port = hostname = uno_path = office_binary_path = None + connection = uno_path = office_binary_path = None for opt, arg in opt_list: - if opt == "--port": - port = arg - elif opt == "--hostname": - hostname = arg + if opt == "--connection": + connection = arg elif opt == "--uno_path": uno_path = arg if uno_path not in sys.path: @@ -40,7 +38,7 @@ def main(): elif opt == "--office_binary_path": office_binary_path = arg - output = json.dumps(test_openoffice(hostname, port, + output = json.dumps(test_openoffice(connection, uno_path, office_binary_path)) sys.stdout.write(output) diff --git a/cloudooo/handler/ooo/helper/unoconverter.py b/cloudooo/handler/ooo/helper/unoconverter.py index 784cbf9..6cd03fb 100755 --- a/cloudooo/handler/ooo/helper/unoconverter.py +++ b/cloudooo/handler/ooo/helper/unoconverter.py @@ -298,7 +298,7 @@ def main(): opt_list, arg_list = getopt(sys.argv[1:], "h", ["help", "test", "convert", "getmetadata", "setmetadata", "uno_path=", "office_binary_path=", - "hostname=", "port=", "source_format=", + "connection=", "source_format=", "document_url=", "destination_format=", "mimemapper=", "metadata=", "refresh=", "unomimemapper_bin="]) @@ -313,15 +313,13 @@ def main(): import json except ImportError: import simplejson as json - hostname = port = document_url = office_binary_path = uno_path =\ + connection = document_url = office_binary_path = uno_path =\ destination_format = source_format = refresh = metadata = mimemapper = None for opt, arg in iter(opt_list): if opt in ('-h', '--help'): help() - elif opt == '--hostname': - hostname = arg - elif opt == '--port': - port = arg + elif opt == '--connection': + connection = arg elif opt == '--document_url': document_url = arg elif opt == '--office_binary_path': @@ -340,7 +338,7 @@ def main(): elif opt == '--mimemapper': mimemapper = json.loads(arg) - service_manager = helper_util.getServiceManager(hostname, port, + service_manager = helper_util.getServiceManager(connection, uno_path, office_binary_path) if '--getmetadata' in param_list and '--convert' in param_list: with UnoConverter(service_manager, document_url, source_format, refresh) as unoconverter: diff --git a/cloudooo/handler/ooo/helper/unomimemapper.py b/cloudooo/handler/ooo/helper/unomimemapper.py index a2f3ee7..e550e63 100755 --- a/cloudooo/handler/ooo/helper/unomimemapper.py +++ b/cloudooo/handler/ooo/helper/unomimemapper.py @@ -66,9 +66,9 @@ Options: class UnoMimemapper(object): """ """ - def __init__(self, hostname, port, uno_path=None, office_binary_path=None): + def __init__(self, connection, uno_path=None, office_binary_path=None): """ Receives hostname and port from openoffice and create a service manager""" - self.service_manager = helper_util.getServiceManager(hostname, port, + self.service_manager = helper_util.getServiceManager(connection, uno_path, office_binary_path) @@ -111,7 +111,7 @@ def main(): try: opt_list, arg_list = getopt(sys.argv[1:], "h", ["help", "uno_path=", "office_binary_path=", - "hostname=", "port="]) + "connection="]) except GetoptError as msg: msg = msg.msg + "\nUse --help or -h\n" sys.stderr.write(msg) @@ -120,7 +120,7 @@ def main(): if not opt_list: help() - port = hostname = uno_path = office_binary_path = None + connection = uno_path = office_binary_path = None for opt, arg in opt_list: if opt in ("-h", "--help"): help() @@ -128,12 +128,10 @@ def main(): uno_path = arg elif opt == "--office_binary_path": office_binary_path = arg - elif opt == '--hostname': - hostname = arg - elif opt == "--port": - port = arg + elif opt == '--connection': + connection = arg - mimemapper = UnoMimemapper(hostname, port, uno_path, office_binary_path) + mimemapper = UnoMimemapper(connection, uno_path, office_binary_path) filter_dict = mimemapper.getFilterDict() type_dict = mimemapper.getTypeDict() diff --git a/cloudooo/handler/ooo/mimemapper.py b/cloudooo/handler/ooo/mimemapper.py index 256869d..c8a1a14 100644 --- a/cloudooo/handler/ooo/mimemapper.py +++ b/cloudooo/handler/ooo/mimemapper.py @@ -88,7 +88,7 @@ class MimeMapper(object): """Verify if filters were loaded""" return self._loaded - def loadFilterList(self, hostname, port, **kw): + def loadFilterList(self, connection, **kw): """Load all filters of openoffice. Keyword arguments: hostname -- host of OpenOffice @@ -122,8 +122,7 @@ class MimeMapper(object): path.join("helper", "unomimemapper.py")), "--uno_path=%s" % uno_path, "--office_binary_path=%s" % office_binary_path, - "--hostname=%s" % hostname, - "--port=%s" % port] + "--connection=%s" % connection] process = Popen(command, stdout=PIPE, stderr=PIPE, close_fds=True) stdout, stderr = process.communicate() diff --git a/cloudooo/handler/ooo/tests/testOooMimemapper.py b/cloudooo/handler/ooo/tests/testOooMimemapper.py index 38bdfcc..d11b21f 100644 --- a/cloudooo/handler/ooo/tests/testOooMimemapper.py +++ b/cloudooo/handler/ooo/tests/testOooMimemapper.py @@ -150,9 +150,7 @@ class TestMimeMapper(HandlerTestCase): """Mimemapper is created and load uno path.""" self.mimemapper = MimeMapper() openoffice.acquire() - hostname, port = openoffice.getAddress() - self.mimemapper.loadFilterList(hostname, - port, + self.mimemapper.loadFilterList(openoffice.getConnection(), python_path=self.python_path) openoffice.release() diff --git a/cloudooo/handler/ooo/tests/testOooMonitorMemory.py b/cloudooo/handler/ooo/tests/testOooMonitorMemory.py index 1d16d4c..4377b75 100644 --- a/cloudooo/handler/ooo/tests/testOooMonitorMemory.py +++ b/cloudooo/handler/ooo/tests/testOooMonitorMemory.py @@ -32,7 +32,6 @@ from cloudooo.handler.ooo.application.openoffice import openoffice from cloudooo.handler.ooo.monitor.memory import MonitorMemory from psutil import Process from types import IntType -from cloudooo.tests.handlerTestCase import make_suite OPENOFFICE = True diff --git a/cloudooo/handler/ooo/tests/testOooUnoConverter.py b/cloudooo/handler/ooo/tests/testOooUnoConverter.py index adb8f71..c763d30 100644 --- a/cloudooo/handler/ooo/tests/testOooUnoConverter.py +++ b/cloudooo/handler/ooo/tests/testOooUnoConverter.py @@ -47,7 +47,7 @@ class TestUnoConverter(HandlerTestCase): def afterSetUp(self): """ """ openoffice.acquire() - self.hostname, self.port = openoffice.getAddress() + self.connection = openoffice.getConnection() data = open("data/test.odt", 'r').read() self.document = FileSystemDocument(self.tmp_url, data, 'odt') @@ -69,8 +69,7 @@ class TestUnoConverter(HandlerTestCase): "--convert", "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path, - "--hostname=%s" % self.hostname, - "--port=%s" % self.port, + "--connection=%s" % self.connection, "--document_url=%s" % self.document.getUrl(), "--destination_format=%s" % "doc", "--source_format=%s" % "odt", diff --git a/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py b/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py index 87751e8..8848a27 100644 --- a/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py +++ b/cloudooo/handler/ooo/tests/testOooUnoMimemapper.py @@ -54,36 +54,13 @@ class TestUnoMimeMapper(HandlerTestCase): def testCreateLocalAttributes(self): """Test if filters returns correctly the filters and types in dict""" - hostname, host = openoffice.getAddress() python = path.join(self.office_binary_path, "python") command = [path.exists(python) and python or "python", pkg_resources.resource_filename(self.package_namespace, "/helper/unomimemapper.py"), "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path, - "--hostname=%s" % self.hostname, - "--port=%s" % self.openoffice_port] - process = Popen(command, stdout=PIPE, stderr=PIPE) - stdout, stderr = process.communicate() - self.assertEquals(process.returncode, -15) - self.assertEquals(stderr, '') - filter_dict, type_dict = json.loads(stdout) - self.assertTrue('filter_dict' in locals()) - self.assertTrue('type_dict' in locals()) - self.assertNotEquals(filter_dict.get('writer8'), None) - self.assertEquals(type_dict.get('writer8').get('Name'), 'writer8') - self.assertNotEquals(filter_dict.get('writer8'), None) - self.assertEquals(type_dict.get('writer8').get('PreferredFilter'), 'writer8') - self.assertEquals(stderr, '') - - def testCallUnoMimemapperOnlyHostNameAndPort(self): - """ Test call unomimemapper without uno_path and office_binary_path""" - hostname, host = openoffice.getAddress() - command = [path.join(self.office_binary_path, "python"), - pkg_resources.resource_filename(self.package_namespace, - "/helper/unomimemapper.py"), - "--hostname=%s" % self.hostname, - "--port=%s" % self.openoffice_port] + "--connection=%s" % openoffice.getConnection()] process = Popen(command, stdout=PIPE, stderr=PIPE) stdout, stderr = process.communicate() self.assertEquals(process.returncode, -3) @@ -99,8 +76,7 @@ class TestUnoMimeMapper(HandlerTestCase): def testWithoutOpenOffice(self): """Test when the openoffice is stopped""" - error_msg = "couldn\'t connect to socket (Success)\n" - hostname, host = openoffice.getAddress() + error_msg = "helper_util.com.sun.star.connection.NoConnectException: Connector : couldn't connect to " openoffice.stop() python = path.join(self.office_binary_path, "python") command = [path.exists(python) and python or "python", @@ -108,13 +84,12 @@ class TestUnoMimeMapper(HandlerTestCase): "/helper/unomimemapper.py"), "--uno_path=%s" % self.uno_path, "--office_binary_path=%s" % self.office_binary_path, - "--hostname=%s" % self.hostname, - "--port=%s" % self.openoffice_port] + "--connection=%s" % openoffice.getConnection()] process = Popen(command, stdout=PIPE, stderr=PIPE) stdout, stderr = process.communicate() self.assertEquals(process.returncode, -3) self.assertEquals(stdout, '') - self.assertTrue(stderr.endswith(error_msg), stderr) + self.assertTrue(error_msg in stderr, stderr) openoffice.start() diff --git a/cloudooo/handler/ooo/util.py b/cloudooo/handler/ooo/util.py index ac48de9..b594419 100644 --- a/cloudooo/handler/ooo/util.py +++ b/cloudooo/handler/ooo/util.py @@ -26,8 +26,6 @@ # ############################################################################## -from socket import socket, error -from errno import EADDRINUSE from time import sleep from os import remove from shutil import rmtree @@ -41,19 +39,6 @@ def removeDirectory(path): except OSError, msg: logger.error(msg) - -def socketStatus(hostname, port): - """Verify if the address is busy.""" - try: - socket().bind((hostname, port),) - # False if the is free - return False - except error, (num, err): - if num == EADDRINUSE: - # True if the isn't free - return True - - def waitStartDaemon(daemon, attempts): """Wait a certain time to start the daemon.""" for num in range(attempts): diff --git a/cloudooo/tests/handlerTestCase.py b/cloudooo/tests/handlerTestCase.py index e8207f4..fd26165 100644 --- a/cloudooo/tests/handlerTestCase.py +++ b/cloudooo/tests/handlerTestCase.py @@ -58,7 +58,7 @@ def startFakeEnvironment(start_openoffice=True, conf_path=None): uno_path = config.get("app:main", "uno_path") working_path = config.get("app:main", "working_path") hostname = config.get("server:main", "host") - openoffice_port = int(config.get("app:main", "openoffice_port")) + openoffice_port = int(config.get("app:main", "openoffice_port")) + 1 office_binary_path = config.get("app:main", "office_binary_path") environment_dict = {} for item in config.options("app:main"): @@ -93,11 +93,10 @@ def startFakeEnvironment(start_openoffice=True, conf_path=None): environment_dict) openoffice.start() openoffice.acquire() - hostname, port = openoffice.getAddress() kw = dict(uno_path=config.get("app:main", "uno_path"), office_binary_path=config.get("app:main", "office_binary_path")) if not mimemapper.isLoaded(): - mimemapper.loadFilterList(hostname, port, **kw) + mimemapper.loadFilterList(openoffice.getConnection(), **kw) openoffice.release() return openoffice @@ -110,8 +109,9 @@ def stopFakeEnvironment(stop_openoffice=True): if 1: from cloudooo.handler.ooo.application.openoffice import OpenOffice - from cloudooo.handler.ooo.util import waitStartDaemon, waitStopDaemon + from cloudooo.handler.ooo.util import waitStartDaemon from subprocess import Popen, PIPE + from time import sleep # patch OpenOffice._startProcess not to put bogus output to stderr, # that prevents detecting the end of unit test. @@ -119,14 +119,16 @@ if 1: """Start OpenOffice.org process""" for i in range(5): self.stop() - waitStopDaemon(self, self.timeout) self.process = Popen(command, stderr=PIPE, close_fds=True, env=env) - if not waitStartDaemon(self, self.timeout): + sleep(1) + if self.process.poll() is not None: + # process already terminated so + # rerun continue - if self._testOpenOffice(self.hostname, self.port): - return + if waitStartDaemon(self, self.timeout - 1): + break OpenOffice._startProcess = _startProcess -- 2.30.9 From 66e7ad71977c7358f35597aca4378cd77f703e90 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Fri, 24 May 2019 03:13:06 +0300 Subject: [PATCH 10/20] improve logging --- .../handler/ooo/application/application.py | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 0f04eb4..8591337 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -46,36 +46,36 @@ class Application(object): self.getAddress()[-1], self.pid())) - def stopProcess(self, process_pid): - """Stop the process""" - error = False - logger.debug("Stop Pid - %s", process_pid) - returncode = None - try: - process = Process(process_pid) - cmdline = " ".join(process.cmdline()) - process.terminate() - returncode = process.wait(self.timeout) - except NoSuchProcess: - pass - except TimeoutExpired: - error = True - logger.error("Process %s survived SIGTERM after %s", process_pid, self.timeout) + def stop(self): + if hasattr(self, 'process'): + error = False + process_pid = self.process.pid + logger.debug("Stop Pid - %s", process_pid) + returncode = None try: - process.kill() + process = Process(process_pid) + cmdline = " ".join(process.cmdline()) + process.terminate() returncode = process.wait(self.timeout) except NoSuchProcess: pass except TimeoutExpired: - logger.error("Process %s survived SIGKILL after %s", process_pid, self.timeout) - if error and returncode: - logger.error("Process %s cmdline: %s ended with returncode %s", process_pid, cmdline, returncode) - elif returncode != 0: - logger.debug("Process %s ended with returncode %s", process_pid, returncode) + error = True + logger.error("Process %s survived SIGTERM after %s", process_pid, self.timeout) + try: + process.kill() + returncode = process.wait(self.timeout) + except NoSuchProcess: + pass + except TimeoutExpired: + logger.error("Process %s survived SIGKILL after %s", process_pid, self.timeout) - def stop(self): - if hasattr(self, 'process'): - self.stopProcess(self.process.pid) + if returncode is None: + returncode = self.process.returncode + if error and returncode: + logger.error("Process %s cmdline: %s ended with returncode %s", process_pid, cmdline, returncode) + elif returncode != 0: + logger.debug("Process %s ended with returncode %s", process_pid, returncode) delattr(self, "process") def loadSettings(self, hostname, port, path_run_dir, **kwargs): -- 2.30.9 From edacbe102944af5b810eb2d3ead73f4f7786732f Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 30 May 2019 12:45:28 +0300 Subject: [PATCH 11/20] improve logging --- .../handler/ooo/application/application.py | 21 ++++++++++++++-- .../handler/ooo/application/openoffice.py | 2 ++ cloudooo/tests/handlerTestCase.py | 25 ------------------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 8591337..88280d1 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -50,8 +50,14 @@ class Application(object): if hasattr(self, 'process'): error = False process_pid = self.process.pid + returncode = self.process.poll() + # libreoffice daemon want go to background + # so it return 0, so ignore 0 + if returncode is not None and returncode != 0: + self.daemonOutputLog() + logger.debug("Process %s already ended with returncode %s", process_pid, returncode) + return False logger.debug("Stop Pid - %s", process_pid) - returncode = None try: process = Process(process_pid) cmdline = " ".join(process.cmdline()) @@ -74,10 +80,21 @@ class Application(object): returncode = self.process.returncode if error and returncode: logger.error("Process %s cmdline: %s ended with returncode %s", process_pid, cmdline, returncode) - elif returncode != 0: + elif returncode is not None and returncode != 0: + self.daemonOutputLog() logger.debug("Process %s ended with returncode %s", process_pid, returncode) delattr(self, "process") + def daemonOutputLog(self): + if hasattr(self, 'process'): + process_pid = self.process.pid + stdout = self.process.stdout.read() + if stdout: + logger.debug("Process %s stdout: %s", process_pid, stdout) + stderr = self.process.stderr.read() + if stderr: + logger.error("Process %s stderr: %s", process_pid, stderr) + def loadSettings(self, hostname, port, path_run_dir, **kwargs): """Define attributes for application instance Keyword arguments: diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 34c126a..9876a7d 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -122,6 +122,8 @@ class OpenOffice(Application): for i in range(5): self.stop() self.process = Popen(command, + stdout=PIPE, + stderr=PIPE, close_fds=True, env=env) sleep(1) diff --git a/cloudooo/tests/handlerTestCase.py b/cloudooo/tests/handlerTestCase.py index fd26165..116f794 100644 --- a/cloudooo/tests/handlerTestCase.py +++ b/cloudooo/tests/handlerTestCase.py @@ -107,31 +107,6 @@ def stopFakeEnvironment(stop_openoffice=True): openoffice.stop() return True -if 1: - from cloudooo.handler.ooo.application.openoffice import OpenOffice - from cloudooo.handler.ooo.util import waitStartDaemon - from subprocess import Popen, PIPE - from time import sleep - - # patch OpenOffice._startProcess not to put bogus output to stderr, - # that prevents detecting the end of unit test. - def _startProcess(self, command, env): - """Start OpenOffice.org process""" - for i in range(5): - self.stop() - self.process = Popen(command, stderr=PIPE, - close_fds=True, - env=env) - sleep(1) - if self.process.poll() is not None: - # process already terminated so - # rerun - continue - if waitStartDaemon(self, self.timeout - 1): - break - - OpenOffice._startProcess = _startProcess - class HandlerTestCase(unittest.TestCase): """Test Case to load cloudooo conf.""" -- 2.30.9 From e4ba9a634e6470aa903f67f461db6709275cbfd7 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 30 May 2019 19:27:42 +0300 Subject: [PATCH 12/20] improve logging --- cloudooo/handler/ooo/application/application.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 88280d1..67c57d6 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -58,6 +58,7 @@ class Application(object): logger.debug("Process %s already ended with returncode %s", process_pid, returncode) return False logger.debug("Stop Pid - %s", process_pid) + cmdline = "" try: process = Process(process_pid) cmdline = " ".join(process.cmdline()) -- 2.30.9 From 0ec77252822779f67b07a6a27cfd2d08b1b15524 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Sat, 25 May 2019 02:05:20 +0300 Subject: [PATCH 13/20] fix memory monitor --- .../handler/ooo/application/application.py | 6 ++++- .../handler/ooo/application/openoffice.py | 6 ++--- cloudooo/handler/ooo/monitor/memory.py | 24 +++++++++++-------- cloudooo/handler/ooo/monitor/timeout.py | 4 +++- .../handler/ooo/tests/testOooMonitorMemory.py | 6 ++--- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index 67c57d6..c1f4ebf 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -46,8 +46,11 @@ class Application(object): self.getAddress()[-1], self.pid())) - def stop(self): + def stop(self, pid=None): if hasattr(self, 'process'): + if pid is not None and self.process.pid != pid: + # pid already stopped + return False error = False process_pid = self.process.pid returncode = self.process.poll() @@ -85,6 +88,7 @@ class Application(object): self.daemonOutputLog() logger.debug("Process %s ended with returncode %s", process_pid, returncode) delattr(self, "process") + return True def daemonOutputLog(self): if hasattr(self, 'process'): diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 9876a7d..d7ce5f2 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -164,11 +164,11 @@ class OpenOffice(Application): self._cleanRequest() Application.start(self) - def stop(self): + def stop(self, pid=None): """Stop the instance by pid. By the default the signal is 15.""" - Application.stop(self) - self._cleanRequest() + if Application.stop(self, pid=pid): + self._cleanRequest() def isLocked(self): """Verify if OOo instance is being used.""" diff --git a/cloudooo/handler/ooo/monitor/memory.py b/cloudooo/handler/ooo/monitor/memory.py index 1308284..6f746dd 100644 --- a/cloudooo/handler/ooo/monitor/memory.py +++ b/cloudooo/handler/ooo/monitor/memory.py @@ -44,15 +44,18 @@ class MonitorMemory(Monitor, Process): Process.__init__(self) self.limit = limit_memory_usage - def create_process(self): - self.process = psutil.Process(int(self.openoffice.pid())) + def create_process(self, pid): + if not hasattr(self, 'process') or \ + self.process.pid != int(pid): + self.process = psutil.Process(pid) + return self.process - def get_memory_usage(self): + def get_memory_usage(self, pid): + if pid is None: + return 0 try: - if not hasattr(self, 'process') or \ - self.process.pid != int(self.openoffice.pid()): - self.create_process() - return self.process.memory_info().rss / (1024 * 1024) + process = self.create_process(pid) + return process.memory_info().rss / (1024 * 1024) except TypeError: logger.debug("OpenOffice is stopped") return 0 @@ -69,9 +72,10 @@ class MonitorMemory(Monitor, Process): self.status_flag = True logger.debug("Start MonitorMemory") while self.status_flag: - if self.get_memory_usage() > self.limit: - logger.debug("Stopping OpenOffice") - self.openoffice.stop() + pid = self.openoffice.pid() + if self.get_memory_usage(pid) > self.limit: + logger.debug("Stopping OpenOffice on memory limit increase") + self.openoffice.stop(pid=pid) sleep(self.interval) logger.debug("Stop MonitorMemory") diff --git a/cloudooo/handler/ooo/monitor/timeout.py b/cloudooo/handler/ooo/monitor/timeout.py index dcaf5aa..bf5605a 100644 --- a/cloudooo/handler/ooo/monitor/timeout.py +++ b/cloudooo/handler/ooo/monitor/timeout.py @@ -50,7 +50,9 @@ class MonitorTimeout(Monitor, Process): sleep(self.interval) if self.openoffice.isLocked(): logger.debug("Stop OpenOffice - Port %s - Pid %s" % (port, pid)) - self.openoffice.stop() + # using pid is not necessary here + # but safe if incorrect use + self.openoffice.stop(pid=pid) def terminate(self): """Stop the process""" diff --git a/cloudooo/handler/ooo/tests/testOooMonitorMemory.py b/cloudooo/handler/ooo/tests/testOooMonitorMemory.py index 4377b75..2c1ef54 100644 --- a/cloudooo/handler/ooo/tests/testOooMonitorMemory.py +++ b/cloudooo/handler/ooo/tests/testOooMonitorMemory.py @@ -90,7 +90,7 @@ class TestMonitorMemory(unittest.TestCase): def testCreateProcess(self): """Test if the psutil.Process is create correctly""" self.monitor = MonitorMemory(openoffice, 2, 400) - self.monitor.create_process() + self.monitor.create_process(openoffice.pid()) self.assertTrue(hasattr(self.monitor, 'process')) self.assertEquals(type(self.monitor.process), Process) @@ -98,11 +98,11 @@ class TestMonitorMemory(unittest.TestCase): """Test memory usage""" self.monitor = MonitorMemory(openoffice, 2, 400) openoffice.stop() - memory_usage_int = self.monitor.get_memory_usage() + memory_usage_int = self.monitor.get_memory_usage(openoffice.pid()) self.assertEquals(memory_usage_int, 0) if not openoffice.status(): openoffice.start() - memory_usage_int = self.monitor.get_memory_usage() + memory_usage_int = self.monitor.get_memory_usage(openoffice.pid()) self.assertEquals(type(memory_usage_int), IntType) -- 2.30.9 From 78c4ce1bd546cb4cb6781ef6c9fc3730b36ca481 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Wed, 29 May 2019 16:32:14 +0300 Subject: [PATCH 14/20] add disabled logger for tests --- cloudooo/tests/handlerTestCase.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cloudooo/tests/handlerTestCase.py b/cloudooo/tests/handlerTestCase.py index 116f794..8ff91f1 100644 --- a/cloudooo/tests/handlerTestCase.py +++ b/cloudooo/tests/handlerTestCase.py @@ -29,6 +29,7 @@ import unittest import sys +from cloudooo import util from os import environ, path, mkdir, putenv from ConfigParser import ConfigParser from cloudooo.handler.ooo.application.openoffice import openoffice @@ -60,6 +61,8 @@ def startFakeEnvironment(start_openoffice=True, conf_path=None): hostname = config.get("server:main", "host") openoffice_port = int(config.get("app:main", "openoffice_port")) + 1 office_binary_path = config.get("app:main", "office_binary_path") + if 0: + util.configureLogger(debug_mode=True) environment_dict = {} for item in config.options("app:main"): if item.startswith("env-"): -- 2.30.9 From a23689c5bf5980c6c9d4e05b7e5b8fc612402f03 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Wed, 29 May 2019 09:25:07 +0300 Subject: [PATCH 15/20] support libreoffice lockfile --- .../handler/ooo/application/openoffice.py | 17 +++++++- cloudooo/handler/ooo/util.py | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index d7ce5f2..949b6b9 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -34,8 +34,11 @@ from zope.interface import implements from application import Application from cloudooo.interfaces.lockable import ILockable from cloudooo.util import logger +import signal from cloudooo.handler.ooo.util import waitStartDaemon, \ - removeDirectory + removeDirectory, \ + processUsedFilesInPath, \ + kill_procs_tree try: import json except ImportError: @@ -139,6 +142,18 @@ class OpenOffice(Application): """Start Instance.""" self.path_user_installation = join(self.path_run_dir, \ "cloudooo_instance_%s" % self.port) + lock_file = join(self.path_user_installation, '.lock') + if exists(lock_file): + pids = processUsedFilesInPath(self.path_user_installation) + if len(pids) == 0: + logger.debug("Stalled lock file: %s", lock_file) + else: + logger.debug("kill process used workdir: %s", self.path_user_installation) + _, alive = kill_procs_tree(pids, sig=signal.SIGKILL, timeout=self.timeout) + if len(alive) > 0: + logger.error("process blocks worked directory and alive after SIGKILL: %s", alive) + removeDirectory(self.path_user_installation) + if init and exists(self.path_user_installation): removeDirectory(self.path_user_installation) # Create command with all parameters to start the instance diff --git a/cloudooo/handler/ooo/util.py b/cloudooo/handler/ooo/util.py index b594419..01eba24 100644 --- a/cloudooo/handler/ooo/util.py +++ b/cloudooo/handler/ooo/util.py @@ -26,10 +26,13 @@ # ############################################################################## +import os from time import sleep from os import remove from shutil import rmtree from cloudooo.util import logger +from psutil import process_iter, Process, NoSuchProcess, wait_procs +import signal def removeDirectory(path): @@ -58,6 +61,42 @@ def waitStopDaemon(daemon, attempts=5): sleep(1) return False +def processUsedFilesInPath(path): + pids = set() + for p in process_iter(attrs=['open_files', 'memory_maps']): + for f in (p.info['open_files'] or []) + (p.info['memory_maps'] or []): + if f.path.startswith(path): + pids.add(p.pid) + return pids + +def kill_procs_tree(pids, sig=signal.SIGTERM, + timeout=3, on_terminate=None): + pids = set(pids) + children_pids = set(pids) + for pid in pids: + parent = None + try: + parent = Process(pid) + except NoSuchProcess: + pass + if parent: + children = parent.children(recursive=True) + for p in children: + children_pids.add(p.pid) + my_pid = os.getpid() + if my_pid in children_pids: + children_pids.remove(my_pid) + pids = [] + for pid in children_pids: + try: + p = Process(pid) + p.send_signal(sig) + pids.append(p) + except NoSuchProcess: + pass + gone, alive = wait_procs(pids, timeout=timeout, + callback=on_terminate) + return (gone, alive) def remove_file(filepath): try: -- 2.30.9 From 81eba22c3345365c46fc4e2159c314a1e294bb77 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Wed, 29 May 2019 19:22:11 +0300 Subject: [PATCH 16/20] fix start issue --- cloudooo/handler/ooo/application/application.py | 13 ++++++++----- cloudooo/handler/ooo/application/openoffice.py | 11 +++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cloudooo/handler/ooo/application/application.py b/cloudooo/handler/ooo/application/application.py index c1f4ebf..91da2e7 100644 --- a/cloudooo/handler/ooo/application/application.py +++ b/cloudooo/handler/ooo/application/application.py @@ -118,11 +118,14 @@ class Application(object): self.start(init=False) def status(self): - """Check by socket if the openoffice work.""" - pid = self.pid() - if pid is None or not pid_exists(pid): - return False - return True + """Check daemon running.""" + if hasattr(self, 'process'): + returncode = self.process.poll() + if returncode is None or returncode == 0: + return True + if pid_exists(self.process.pid): + return True + return False def getAddress(self): """Return port and hostname of OOo Instance.""" diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 949b6b9..8ad2729 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -133,7 +133,8 @@ class OpenOffice(Application): if waitStartDaemon(self, self.timeout - 1): if self.last_test_error: logger.debug(self.last_test_error) - break + return True + return False def getConnection(self): return self.connection @@ -175,9 +176,11 @@ class OpenOffice(Application): env["HOME"] = self.path_user_installation env["TMP"] = self.path_user_installation env["TMPDIR"] = self.path_user_installation - self._startProcess(self.command, env) - self._cleanRequest() - Application.start(self) + if self._startProcess(self.command, env): + self._cleanRequest() + logger.debug("%s libreoffice PID: %s started", self.getConnection(), self.pid()) + else: + logger.error("%s libreoffice not started", self.getConnection()) def stop(self, pid=None): """Stop the instance by pid. By the default -- 2.30.9 From 309a413b382004ada0268e21681f3d564aab93e7 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Thu, 30 May 2019 17:16:50 +0300 Subject: [PATCH 17/20] correctly terminate libreoffice daemon --- .../handler/ooo/application/openoffice.py | 62 ++++++++++++++----- .../handler/ooo/helper/openoffice_tester.py | 20 ++++-- cloudooo/handler/ooo/monitor/memory.py | 2 +- cloudooo/handler/ooo/monitor/timeout.py | 2 +- 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 8ad2729..71d3df0 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -63,10 +63,7 @@ class OpenOffice(Application): self.last_test_error = None self._cleanRequest() - def _testOpenOffice(self): - """Test if OpenOffice was started correctly""" - logger.debug("Test OpenOffice %s - Pid %s" % (self.getConnection(), - self.pid())) + def _run_libreoffice_tester(self, args=[]): python = join(self.office_binary_path, "python") args = [exists(python) and python or "python", pkg_resources.resource_filename("cloudooo", @@ -74,18 +71,41 @@ class OpenOffice(Application): "helper", "openoffice_tester.py")), "--connection=%s" % self.getConnection(), "--uno_path=%s" % self.uno_path, - "--office_binary_path=%s" % self.office_binary_path] + "--office_binary_path=%s" % self.office_binary_path] + args stdout, stderr = Popen(args, stdout=PIPE, stderr=PIPE, close_fds=True).communicate() if stdout == "": logger.error("openoffice_tester.py cmdline: %s", " ".join(args)) logger.error("openoffice_tester.py stdout: %s", stdout) logger.error("openoffice_tester.py stderr: %s", stderr) - return False + return False, stderr stdout = json.loads(stdout) + return stdout, stderr + + def _testOpenOffice(self): + """Test if LibreOffice was started correctly""" + logger.debug("%s instance testing - Pid %s" % (self.getConnection(), + self.pid())) + stdout, stderr = self._run_libreoffice_tester() self.last_test_error = stderr if stdout == True: - logger.debug("Instance %s works" % self.getConnection()) + logger.debug("%s instance works" % self.getConnection()) + return stdout + + def _terminate(self): + """Send LibreOffice daemon terminate command""" + logger.debug("%s instance terminating - Pid %s" % (self.getConnection(), + self.pid())) + stdout, stderr = self._run_libreoffice_tester(["--terminate"]) + if stderr: + logger.error(stderr) + if stdout == True: + for num in range(5): + if not exists(self.lock_file): + logger.debug("%s terminate" % self.getConnection()) + return stdout + sleep(1) + logger.error("%s can not terminate" % self.getConnection()) return stdout def _cleanRequest(self): @@ -104,6 +124,9 @@ class OpenOffice(Application): if environment_dict is None: environment_dict = {} Application.loadSettings(self, hostname, port, path_run_dir) + self.path_user_installation = join(self.path_run_dir, \ + "cloudooo_instance_%s" % self.port) + self.lock_file = join(self.path_user_installation, '.lock') self.office_binary_path = office_binary_path self.uno_path = uno_path self.default_language = default_language @@ -141,13 +164,10 @@ class OpenOffice(Application): def start(self, init=True): """Start Instance.""" - self.path_user_installation = join(self.path_run_dir, \ - "cloudooo_instance_%s" % self.port) - lock_file = join(self.path_user_installation, '.lock') - if exists(lock_file): + if exists(self.lock_file): pids = processUsedFilesInPath(self.path_user_installation) if len(pids) == 0: - logger.debug("Stalled lock file: %s", lock_file) + logger.debug("Stalled lock file: %s", self.lock_file) else: logger.debug("kill process used workdir: %s", self.path_user_installation) _, alive = kill_procs_tree(pids, sig=signal.SIGKILL, timeout=self.timeout) @@ -182,12 +202,22 @@ class OpenOffice(Application): else: logger.error("%s libreoffice not started", self.getConnection()) - def stop(self, pid=None): - """Stop the instance by pid. By the default - the signal is 15.""" - if Application.stop(self, pid=pid): + def reset(self, *args, **kw): + if Application.stop(self, *args, **kw): self._cleanRequest() + def stop(self, pid=None): + if hasattr(self, 'process'): + if pid is not None and self.process.pid != pid: + return False + returncode = self.process.poll() + if returncode is None or returncode == 0: + self._terminate() + if Application.stop(self, pid=pid): + self._cleanRequest() + return True + return False + def isLocked(self): """Verify if OOo instance is being used.""" return self._lock.locked() diff --git a/cloudooo/handler/ooo/helper/openoffice_tester.py b/cloudooo/handler/ooo/helper/openoffice_tester.py index 74887ff..048b12a 100755 --- a/cloudooo/handler/ooo/helper/openoffice_tester.py +++ b/cloudooo/handler/ooo/helper/openoffice_tester.py @@ -9,10 +9,17 @@ except ImportError: import simplejson as json -def test_openoffice(connection, uno_path, office_binary_path): +def test_openoffice(connection, uno_path, office_binary_path, terminate=False): import pyuno try: - helper_util.getServiceManager(connection, uno_path, office_binary_path) + sm = helper_util.getServiceManager(connection, uno_path, office_binary_path) + if terminate: + try: + sm.createInstance("com.sun.star.frame.Desktop").terminate() + except pyuno.getClass("com.sun.star.beans.UnknownPropertyException"): + pass + except pyuno.getClass("com.sun.star.lang.DisposedException"): + pass return True except pyuno.getClass("com.sun.star.connection.NoConnectException"): return False @@ -22,12 +29,14 @@ def main(): try: opt_list, arg_list = getopt(sys.argv[1:], "", ["connection=", "uno_path=", - "office_binary_path="]) + "office_binary_path=", + "terminate"]) except GetoptError as e: sys.stderr.write("%s \nUse --connection" % e) sys.exit(2) connection = uno_path = office_binary_path = None + terminate = False for opt, arg in opt_list: if opt == "--connection": connection = arg @@ -37,9 +46,12 @@ def main(): sys.path.append(uno_path) elif opt == "--office_binary_path": office_binary_path = arg + elif opt == "--terminate": + terminate = True output = json.dumps(test_openoffice(connection, - uno_path, office_binary_path)) + uno_path, office_binary_path, + terminate=terminate)) sys.stdout.write(output) if __name__ == "__main__": diff --git a/cloudooo/handler/ooo/monitor/memory.py b/cloudooo/handler/ooo/monitor/memory.py index 6f746dd..482ee0c 100644 --- a/cloudooo/handler/ooo/monitor/memory.py +++ b/cloudooo/handler/ooo/monitor/memory.py @@ -75,7 +75,7 @@ class MonitorMemory(Monitor, Process): pid = self.openoffice.pid() if self.get_memory_usage(pid) > self.limit: logger.debug("Stopping OpenOffice on memory limit increase") - self.openoffice.stop(pid=pid) + self.openoffice.reset(pid=pid) sleep(self.interval) logger.debug("Stop MonitorMemory") diff --git a/cloudooo/handler/ooo/monitor/timeout.py b/cloudooo/handler/ooo/monitor/timeout.py index bf5605a..180899c 100644 --- a/cloudooo/handler/ooo/monitor/timeout.py +++ b/cloudooo/handler/ooo/monitor/timeout.py @@ -52,7 +52,7 @@ class MonitorTimeout(Monitor, Process): logger.debug("Stop OpenOffice - Port %s - Pid %s" % (port, pid)) # using pid is not necessary here # but safe if incorrect use - self.openoffice.stop(pid=pid) + self.openoffice.reset(pid=pid) def terminate(self): """Stop the process""" -- 2.30.9 From 2e1b87a9c134d6d05f5ed5a9b31bc1180a987040 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Fri, 31 May 2019 02:00:14 +0300 Subject: [PATCH 18/20] speedup starting --- cloudooo/handler/ooo/application/openoffice.py | 8 ++++++++ cloudooo/handler/ooo/util.py | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cloudooo/handler/ooo/application/openoffice.py b/cloudooo/handler/ooo/application/openoffice.py index 71d3df0..033c955 100644 --- a/cloudooo/handler/ooo/application/openoffice.py +++ b/cloudooo/handler/ooo/application/openoffice.py @@ -134,6 +134,14 @@ class OpenOffice(Application): self.connection = "socket,host=%s,port=%d" % (self.hostname, self.port) self.connection = "pipe,name=cloudooo_libreoffice_%s" % str(uuid.uuid1()) + def isRunning(self): + if hasattr(self, "process"): + returncode = self.process.poll() + if returncode is not None and returncode != 0: + return False + return True + return False + def status(self): if Application.status(self) and \ self._testOpenOffice(): diff --git a/cloudooo/handler/ooo/util.py b/cloudooo/handler/ooo/util.py index 01eba24..a2928b8 100644 --- a/cloudooo/handler/ooo/util.py +++ b/cloudooo/handler/ooo/util.py @@ -45,10 +45,10 @@ def removeDirectory(path): def waitStartDaemon(daemon, attempts): """Wait a certain time to start the daemon.""" for num in range(attempts): - if daemon.status(): - return True - elif daemon.pid() is None: + if not daemon.isRunning(): return False + elif daemon.status(): + return True sleep(1) return False -- 2.30.9 From 62fa0ab77cb60c8344eac379a6a56bc46d7ed92c Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Fri, 31 May 2019 15:00:27 +0300 Subject: [PATCH 19/20] add testOooUnoOfficeTester.py --- .../ooo/tests/testOooUnoOfficeTester.py | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 cloudooo/handler/ooo/tests/testOooUnoOfficeTester.py diff --git a/cloudooo/handler/ooo/tests/testOooUnoOfficeTester.py b/cloudooo/handler/ooo/tests/testOooUnoOfficeTester.py new file mode 100644 index 0000000..1a17e8c --- /dev/null +++ b/cloudooo/handler/ooo/tests/testOooUnoOfficeTester.py @@ -0,0 +1,153 @@ +############################################################################## +# +# Copyright (c) 2009-2010 Nexedi SA and Contributors. All Rights Reserved. +# Gabriel M. Monnerat +# +# WARNING: This program as such is intended to be used by professional +# programmers who take the whole responsibility of assessing all potential +# consequences resulting from its eventual inadequacies and bugs +# End users who are looking for a ready-to-use solution with commercial +# guarantees and support are strongly adviced to contract a Free Software +# Service Company +# +# This program is Free Software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. +# +############################################################################## + +import json +import pkg_resources +from cloudooo.handler.ooo.application.openoffice import openoffice +from subprocess import Popen, PIPE +from os import environ, path +from cloudooo.tests.handlerTestCase import HandlerTestCase +from time import sleep + +OPENOFFICE = True + + +class TestUnoOfficeTester(HandlerTestCase): + """Test Case to test all features of script openoffice_tester.py""" + + def afterSetUp(self): + """ """ + self.package_namespace = "cloudooo.handler.ooo" + environ['uno_path'] = '' + environ['office_binary_path'] = '' + openoffice.acquire() + + def tearDown(self): + """ """ + environ['uno_path'] = self.uno_path + environ['office_binary_path'] = self.office_binary_path + openoffice.release() + + def testWithOpenOffice(self): + """Test when the openoffice is started""" + python = path.join(self.office_binary_path, "python") + command = [path.exists(python) and python or "python", + pkg_resources.resource_filename(self.package_namespace, + "/helper/openoffice_tester.py"), + "--uno_path=%s" % self.uno_path, + "--office_binary_path=%s" % self.office_binary_path, + "--connection=%s" % openoffice.getConnection()] + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) + self.assertEquals(stderr, '', stderr) + result = json.loads(stdout) + self.assertTrue(result) + + def testWithoutOpenOffice(self): + """Test when the openoffice is stopped""" + openoffice.stop() + python = path.join(self.office_binary_path, "python") + command = [path.exists(python) and python or "python", + pkg_resources.resource_filename(self.package_namespace, + "/helper/openoffice_tester.py"), + "--uno_path=%s" % self.uno_path, + "--office_binary_path=%s" % self.office_binary_path, + "--connection=%s" % openoffice.getConnection()] + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) + self.assertEquals(stderr, '', stderr) + result = json.loads(stdout) + self.assertFalse(result) + openoffice.start() + + def testExceptionNotify(self): + """Test correctly notify about issues""" + exception = "ModuleNotFoundError: No module named" + exception1 = "ImportError:" + command = ["python", + pkg_resources.resource_filename(self.package_namespace, + "/helper/openoffice_tester.py"), + "--uno_path=%s" % self.uno_path + "not_exist_folder", + "--office_binary_path=%s" % self.office_binary_path, + "--connection=%s" % openoffice.getConnection()] + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + self.assertEquals(process.returncode, -3) + self.assertEquals(stdout, '', stdout) + self.assertTrue((exception in stderr) or (exception1 in stderr), stderr) + + def testOpenOfficeTermination(self): + """Test termination when the openoffice is started""" + python = path.join(self.office_binary_path, "python") + self.assertTrue(path.exists(openoffice.lock_file), "libreoffice not started - lock_file is not exist") + command = [path.exists(python) and python or "python", + pkg_resources.resource_filename(self.package_namespace, + "/helper/openoffice_tester.py"), + "--uno_path=%s" % self.uno_path, + "--office_binary_path=%s" % self.office_binary_path, + "--connection=%s" % openoffice.getConnection(), + "--terminate"] + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + for num in range(5): + if not path.exists(openoffice.lock_file): + break + sleep(1) + self.assertEquals(process.returncode, -3) + self.assertEquals(stderr, '', stderr) + result = json.loads(stdout) + self.assertTrue(result) + self.assertFalse(path.exists(openoffice.lock_file), "libreoffice not correctly stopped - lock_file exist") + openoffice.start() + + def testOpenOfficeTerminationIfStoped(self): + """Test termination when the openoffice is stopped""" + openoffice.stop() + self.assertFalse(path.exists(openoffice.lock_file), "libreoffice not correctly stopped - lock_file exist") + python = path.join(self.office_binary_path, "python") + command = [path.exists(python) and python or "python", + pkg_resources.resource_filename(self.package_namespace, + "/helper/openoffice_tester.py"), + "--uno_path=%s" % self.uno_path, + "--office_binary_path=%s" % self.office_binary_path, + "--connection=%s" % openoffice.getConnection(), + "--terminate"] + process = Popen(command, stdout=PIPE, stderr=PIPE) + stdout, stderr = process.communicate() + for num in range(5): + if not path.exists(openoffice.lock_file): + break + sleep(1) + self.assertEquals(process.returncode, -3) + self.assertEquals(stderr, '', stderr) + result = json.loads(stdout) + self.assertFalse(result) + self.assertFalse(path.exists(openoffice.lock_file), "libreoffice not correctly stopped - lock_file exist") + openoffice.start() -- 2.30.9 From 97b7cb2798f5a48508c8ef5491d674bdb7de2fc7 Mon Sep 17 00:00:00 2001 From: Boris Kocherov Date: Fri, 31 May 2019 20:41:29 +0300 Subject: [PATCH 20/20] cleanup handlerTestCase.py --- cloudooo/tests/handlerTestCase.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/cloudooo/tests/handlerTestCase.py b/cloudooo/tests/handlerTestCase.py index 8ff91f1..aab676e 100644 --- a/cloudooo/tests/handlerTestCase.py +++ b/cloudooo/tests/handlerTestCase.py @@ -30,7 +30,7 @@ import unittest import sys from cloudooo import util -from os import environ, path, mkdir, putenv +from os import environ, path, mkdir from ConfigParser import ConfigParser from cloudooo.handler.ooo.application.openoffice import openoffice from cloudooo.handler.ooo.mimemapper import mimemapper @@ -71,18 +71,9 @@ def startFakeEnvironment(start_openoffice=True, conf_path=None): check_folder(working_path, tmp_dir) if not environ.get('uno_path'): environ['uno_path'] = uno_path - office_binary_path = config.get("app:main", "office_binary_path") if not environ.get('office_binary_path'): environ['office_binary_path'] = office_binary_path - if uno_path not in sys.path: - sys.path.append(uno_path) - - fundamentalrc_file = '%s/fundamentalrc' % office_binary_path - if path.exists(fundamentalrc_file) and \ - 'URE_BOOTSTRAP' not in environ: - putenv('URE_BOOTSTRAP', 'vnd.sun.star.pathname:%s' % fundamentalrc_file) - if start_openoffice: default_language = config.get('app:main', 'openoffice_user_interface_language', False, -- 2.30.9