From f77be701a2914cf9bb87044cf26b437713ae6463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 23 Aug 2017 17:03:00 +0200 Subject: [PATCH 1/8] Only fire events once Events.STARTUP has been seen One part of fixing #2090 --- src/octoprint/events.py | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/src/octoprint/events.py b/src/octoprint/events.py index 7f9b818f..6c64d69e 100644 --- a/src/octoprint/events.py +++ b/src/octoprint/events.py @@ -132,9 +132,11 @@ class EventManager(object): self._registeredListeners = collections.defaultdict(list) self._logger = logging.getLogger(__name__) + self._startup_signaled = False self._shutdown_signaled = False self._queue = queue.Queue() + self._held_back = queue.Queue() self._worker = threading.Thread(target=self._work) self._worker.daemon = True @@ -177,7 +179,30 @@ class EventManager(object): payload being a payload object specific to the event. """ - self._queue.put((event, payload)) + send_held_back = False + if event == Events.STARTUP: + self._logger.info("Processing startup event, this is our first event") + self._startup_signaled = True + send_held_back = True + + self._enqueue(event, payload) + + if send_held_back: + self._logger.info("Adding {} events to queue that " + "were held back before startup event".format(self._held_back.qsize())) + while True: + try: + self._queue.put(self._held_back.get()) + except queue.Empty: + break + + def _enqueue(self, event, payload): + if self._startup_signaled: + q = self._queue + else: + q = self._held_back + + q.put((event, payload)) if event == Events.UPDATED_FILES and "type" in payload and payload["type"] == "printables": # when sending UpdatedFiles with type "printables", also send another event with deprecated type "gcode" @@ -185,7 +210,7 @@ class EventManager(object): import copy legacy_payload = copy.deepcopy(payload) legacy_payload["type"] = "gcode" - self._queue.put((event, legacy_payload)) + q.put((event, legacy_payload)) def subscribe(self, event, callback): """ From a35e145649b929f76c1cd94e6ac9ee2cac808759 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 23 Aug 2017 18:18:06 +0200 Subject: [PATCH 2/8] Don't forward events to uninitialized EventPlugin implementations Another part of fixing #2090 --- src/octoprint/events.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/octoprint/events.py b/src/octoprint/events.py index 6c64d69e..3bfd6d9f 100644 --- a/src/octoprint/events.py +++ b/src/octoprint/events.py @@ -163,7 +163,8 @@ class EventManager(object): octoprint.plugin.call_plugin(octoprint.plugin.types.EventHandlerPlugin, "on_event", - args=(event, payload)) + args=(event, payload), + initialized=True) self._logger.info("Event loop shut down") except: self._logger.exception("Ooops, the event bus worker loop crashed") From 12b8a540814b5d89815effe3b3493cc4fc6b321b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 23 Aug 2017 17:06:04 +0200 Subject: [PATCH 3/8] set_close_exec on intermediary server port for unix & windows Using the win32 API it's possible to prevent the intermediary server socket from inheriting itself to subprocesses. So let's use that here. Another bit of the solution for #2090. --- src/octoprint/server/__init__.py | 28 ++++++++++------- src/octoprint/util/platform/__init__.py | 41 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 src/octoprint/util/platform/__init__.py diff --git a/src/octoprint/server/__init__.py b/src/octoprint/server/__init__.py index 61c3bc93..dbc87c19 100644 --- a/src/octoprint/server/__init__.py +++ b/src/octoprint/server/__init__.py @@ -221,14 +221,13 @@ class Server(object): ### IMPORTANT! ### - ### Do not start any subprocesses until the intermediary server shuts down again or they WILL inherit the - ### open port and prevent us from firing up Tornado later. Thanks to close_fds not being available on Popen - ### on Windows if you also want to be able to redirect stdout/stderr/stdin and fnctl also not being available - ### we don't have a good way around this issue besides being careful not to spawn processes here. + ### Best do not start any subprocesses until the intermediary server shuts down again or they MIGHT inherit the + ### open port and prevent us from firing up Tornado later. ### - ### Which kinda sucks tbh. + ### The intermediary server's socket should have the CLOSE_EXEC flag (or its equivalent) set where possible, but + ### we can only do that if fcntl is availabel or we are on Windows, so better safe than sorry. ### - ### See also issue #2035 + ### See also issues #2035 and #2090 # then initialize the plugin manager pluginManager.reload_plugins(startup=True, initialize_implementations=False) @@ -1382,10 +1381,11 @@ class Server(object): bind_and_activate=False) # if possible, make sure our socket's port descriptor isn't handed over to subprocesses - if fcntl is not None and hasattr(fcntl, "FD_CLOEXEC"): - flags = fcntl.fcntl(self._intermediary_server.socket, fcntl.F_GETFD) - flags |= fcntl.FD_CLOEXEC - fcntl.fcntl(self._intermediary_server.socket, fcntl.F_SETFD, flags) + from octoprint.util.platform import set_close_exec + try: + set_close_exec(self._intermediary_server.fileno()) + except: + self._logger.exception("Error while attempting to set_close_exec on intermediary server socket") # then bind the server and have it serve our handler until stopped try: @@ -1395,7 +1395,13 @@ class Server(object): self._intermediary_server.server_close() raise - thread = threading.Thread(target=self._intermediary_server.serve_forever) + def serve(): + try: + self._intermediary_server.serve_forever() + except: + self._logger.exception("Error in intermediary server") + + thread = threading.Thread(target=serve) thread.daemon = True thread.start() diff --git a/src/octoprint/util/platform/__init__.py b/src/octoprint/util/platform/__init__.py new file mode 100644 index 00000000..529c2676 --- /dev/null +++ b/src/octoprint/util/platform/__init__.py @@ -0,0 +1,41 @@ +# coding=utf-8 +from __future__ import absolute_import, division, print_function + +__license__ = 'GNU Affero General Public License http://www.gnu.org/licenses/agpl.html' +__copyright__ = "Copyright (C) 2017 The OctoPrint Project - Released under terms of the AGPLv3 License" + +import sys + +try: + import fcntl +except ImportError: + fcntl = None + +# set_close_exec + +if fcntl is not None and hasattr(fcntl, "FD_CLOEXEC"): + def set_close_exec(handle): + flags = fcntl.fcntl(handle, fcntl.F_GETFD) + flags |= fcntl.FD_CLOEXEC + fcntl.fcntl(handle, fcntl.F_SETFD, flags) + +elif sys.platform == "win32": + def set_close_exec(handle): + import ctypes + import ctypes.wintypes + + # see https://msdn.microsoft.com/en-us/library/ms724935(v=vs.85).aspx + SetHandleInformation = ctypes.windll.kernel32.SetHandleInformation + SetHandleInformation.argtypes = (ctypes.wintypes.HANDLE, ctypes.wintypes.DWORD, ctypes.wintypes.DWORD) + SetHandleInformation.restype = ctypes.c_bool + + HANDLE_FLAG_INHERIT = 0x00000001 + + result = SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 0) + if not result: + raise ctypes.GetLastError() + +else: + def set_close_exec(handle): + # no-op + pass From 2ed36ed383461353d9f7ea704f37acfb2f1b0b49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Wed, 23 Aug 2017 19:07:08 +0200 Subject: [PATCH 4/8] Do not block when processing held back events! --- src/octoprint/events.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/octoprint/events.py b/src/octoprint/events.py index 3bfd6d9f..deef9f83 100644 --- a/src/octoprint/events.py +++ b/src/octoprint/events.py @@ -193,7 +193,7 @@ class EventManager(object): "were held back before startup event".format(self._held_back.qsize())) while True: try: - self._queue.put(self._held_back.get()) + self._queue.put(self._held_back.get(block=False)) except queue.Empty: break From 97cb0088f657c102d335d2efeead65db2d25a772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Thu, 24 Aug 2017 19:16:41 +0200 Subject: [PATCH 5/8] Delay opening of serial.log until first message That way we don't risk running into potential "file busy" issues under windows on log file rollover. --- src/octoprint/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/octoprint/__init__.py b/src/octoprint/__init__.py index 7d6d07ce..785a5e5c 100644 --- a/src/octoprint/__init__.py +++ b/src/octoprint/__init__.py @@ -194,7 +194,8 @@ def init_logging(settings, use_logging_file=True, logging_file=None, default_con "level": "DEBUG", "formatter": "serial", "backupCount": 3, - "filename": os.path.join(settings.getBaseFolder("logs"), "serial.log") + "filename": os.path.join(settings.getBaseFolder("logs"), "serial.log"), + "delay": True } }, "loggers": { From 5989d243d3722a3828aecc097259cbf34ceafddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Fri, 25 Aug 2017 09:47:11 +0200 Subject: [PATCH 6/8] Fix wrapping of temperature controls on touch devices Successfully tested in Chrome (desktop & Android), Firefox (desktop & Android), Safari (desktop & iOS). Closes #2059 --- src/octoprint/static/js/lib/bootstrap/bootstrap.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/octoprint/static/js/lib/bootstrap/bootstrap.js b/src/octoprint/static/js/lib/bootstrap/bootstrap.js index 5ca3cc94..94c4b9f3 100644 --- a/src/octoprint/static/js/lib/bootstrap/bootstrap.js +++ b/src/octoprint/static/js/lib/bootstrap/bootstrap.js @@ -687,7 +687,11 @@ if (!isActive) { if ('ontouchstart' in document.documentElement) { // if mobile we we use a backdrop because click events don't delegate - $('