From 916cf2d439c406dd08310714ae31216608ea6fa6 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Tue, 4 Aug 2020 10:03:41 +0100
Subject: re-implement daemonize (#8011)

This has long been something I've wanted to do. Basically the `Daemonize` code
is both too flexible and not flexible enough, in that it offers a bunch of
features that we don't use (changing UID, closing FDs in the child, logging to
syslog) and doesn't offer a bunch that we could do with (redirecting stdout/err
to a file instead of /dev/null; having the parent not exit until the child is
running).

As a first step, I've lifted the Daemonize code and removed the bits we don't
use. This should be a non-functional change. Fixing everything else will come
later.
---
 synapse/util/daemonize.py | 131 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)
 create mode 100644 synapse/util/daemonize.py

(limited to 'synapse/util')

diff --git a/synapse/util/daemonize.py b/synapse/util/daemonize.py
new file mode 100644
index 0000000000..a7913fa1af
--- /dev/null
+++ b/synapse/util/daemonize.py
@@ -0,0 +1,131 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2012, 2013, 2014 Ilya Otyutskiy <ilya.otyutskiy@icloud.com>
+# Copyright 2020 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import atexit
+import fcntl
+import logging
+import os
+import signal
+import sys
+
+
+def daemonize_process(pid_file: str, logger: logging.Logger, chdir: str = "/") -> None:
+    """daemonize the current process
+
+    This calls fork(), and has the main process exit. When it returns we will be
+    running in the child process.
+    """
+
+    # If pidfile already exists, we should read pid from there; to overwrite it, if
+    # locking will fail, because locking attempt somehow purges the file contents.
+    if os.path.isfile(pid_file):
+        with open(pid_file, "r") as pid_fh:
+            old_pid = pid_fh.read()
+
+    # Create a lockfile so that only one instance of this daemon is running at any time.
+    try:
+        lock_fh = open(pid_file, "w")
+    except IOError:
+        print("Unable to create the pidfile.")
+        sys.exit(1)
+
+    try:
+        # Try to get an exclusive lock on the file. This will fail if another process
+        # has the file locked.
+        fcntl.flock(lock_fh, fcntl.LOCK_EX | fcntl.LOCK_NB)
+    except IOError:
+        print("Unable to lock on the pidfile.")
+        # We need to overwrite the pidfile if we got here.
+        #
+        # XXX better to avoid overwriting it, surely. this looks racey as the pid file
+        # could be created between us trying to read it and us trying to lock it.
+        with open(pid_file, "w") as pid_fh:
+            pid_fh.write(old_pid)
+        sys.exit(1)
+
+    # Fork, creating a new process for the child.
+    process_id = os.fork()
+
+    if process_id != 0:
+        # parent process
+        sys.exit(0)
+
+    # This is the child process. Continue.
+
+    # Stop listening for signals that the parent process receives.
+    # This is done by getting a new process id.
+    # setpgrp() is an alternative to setsid().
+    # setsid puts the process in a new parent group and detaches its controlling
+    # terminal.
+
+    os.setsid()
+
+    # point stdin, stdout, stderr at /dev/null
+    devnull = "/dev/null"
+    if hasattr(os, "devnull"):
+        # Python has set os.devnull on this system, use it instead as it might be
+        # different than /dev/null.
+        devnull = os.devnull
+
+    devnull_fd = os.open(devnull, os.O_RDWR)
+    os.dup2(devnull_fd, 0)
+    os.dup2(devnull_fd, 1)
+    os.dup2(devnull_fd, 2)
+    os.close(devnull_fd)
+
+    # now that we have redirected stderr to /dev/null, any uncaught exceptions will
+    # get sent to /dev/null, so make sure we log them.
+    #
+    # (we don't normally expect reactor.run to raise any exceptions, but this will
+    # also catch any other uncaught exceptions before we get that far.)
+
+    def excepthook(type_, value, traceback):
+        logger.critical("Unhanded exception", exc_info=(type_, value, traceback))
+
+    sys.excepthook = excepthook
+
+    # Set umask to default to safe file permissions when running as a root daemon. 027
+    # is an octal number which we are typing as 0o27 for Python3 compatibility.
+    os.umask(0o27)
+
+    # Change to a known directory. If this isn't done, starting a daemon in a
+    # subdirectory that needs to be deleted results in "directory busy" errors.
+    os.chdir(chdir)
+
+    try:
+        lock_fh.write("%s" % (os.getpid()))
+        lock_fh.flush()
+    except IOError:
+        logger.error("Unable to write pid to the pidfile.")
+        print("Unable to write pid to the pidfile.")
+        sys.exit(1)
+
+    # write a log line on SIGTERM.
+    def sigterm(signum, frame):
+        logger.warning("Caught signal %s. Stopping daemon." % signum)
+        sys.exit(0)
+
+    signal.signal(signal.SIGTERM, sigterm)
+
+    # Cleanup pid file at exit.
+    def exit():
+        logger.warning("Stopping daemon.")
+        os.remove(pid_file)
+        sys.exit(0)
+
+    atexit.register(exit)
+
+    logger.warning("Starting daemon.")
-- 
cgit 1.4.1


From 0a86850ba37470837cbd19b33679763851c13b73 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Wed, 5 Aug 2020 09:35:17 +0100
Subject: Stop the parent process flushing the logs on exit (#8012)

This solves the problem that the first few lines are logged twice on matrix.org. Hopefully the comments explain it.
---
 changelog.d/8011.bugfix   |  1 +
 changelog.d/8011.misc     |  1 -
 changelog.d/8012.bugfix   |  1 +
 synapse/util/daemonize.py | 10 ++++++++--
 4 files changed, 10 insertions(+), 3 deletions(-)
 create mode 100644 changelog.d/8011.bugfix
 delete mode 100644 changelog.d/8011.misc
 create mode 100644 changelog.d/8012.bugfix

(limited to 'synapse/util')

diff --git a/changelog.d/8011.bugfix b/changelog.d/8011.bugfix
new file mode 100644
index 0000000000..c673040de9
--- /dev/null
+++ b/changelog.d/8011.bugfix
@@ -0,0 +1 @@
+Fix a long-standing bug which caused two copies of some log lines to be written when synctl was used along with a MemoryHandler logger.
diff --git a/changelog.d/8011.misc b/changelog.d/8011.misc
deleted file mode 100644
index dfeb4bdaf1..0000000000
--- a/changelog.d/8011.misc
+++ /dev/null
@@ -1 +0,0 @@
-Replace daemonize library with a local implementation.
diff --git a/changelog.d/8012.bugfix b/changelog.d/8012.bugfix
new file mode 100644
index 0000000000..c673040de9
--- /dev/null
+++ b/changelog.d/8012.bugfix
@@ -0,0 +1 @@
+Fix a long-standing bug which caused two copies of some log lines to be written when synctl was used along with a MemoryHandler logger.
diff --git a/synapse/util/daemonize.py b/synapse/util/daemonize.py
index a7913fa1af..23393cf49b 100644
--- a/synapse/util/daemonize.py
+++ b/synapse/util/daemonize.py
@@ -60,8 +60,14 @@ def daemonize_process(pid_file: str, logger: logging.Logger, chdir: str = "/") -
     process_id = os.fork()
 
     if process_id != 0:
-        # parent process
-        sys.exit(0)
+        # parent process: exit.
+
+        # we use os._exit to avoid running the atexit handlers. In particular, that
+        # means we don't flush the logs. This is important because if we are using
+        # a MemoryHandler, we could have logs buffered which are now buffered in both
+        # the main and the child process, so if we let the main process flush the logs,
+        # we'll get two copies.
+        os._exit(0)
 
     # This is the child process. Continue.
 
-- 
cgit 1.4.1