From bf5c9706d9053ffad05fc12eca71b8d441fa9306 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Thu, 12 Jan 2017 10:32:52 +0000
Subject: [PATCH] Remove full_twisted_stacktraces option

The debug 'full_twisted_stacktraces' flag caused synapse to rewrite
twisted deferreds to always fire the callback on the next reactor tick.
This was to force the deferred to always store the stacktraces on
exceptions, and thus be more likely to have a full stacktrace when it
reaches the final error handlers and gets printed to the logs.

Dynamically rewriting things is generally bad, and in particular this
change violates assumptions of various bits of Twisted. This wouldn't
necessarily be so bad, but it turns out this option has been turned on
on some production servers.

Turning the option can cause e.g. #1778.

For now, lets just entirely nuke this option.
---
 synapse/config/logger.py |  8 -----
 synapse/util/debug.py    | 71 ----------------------------------------
 2 files changed, 79 deletions(-)
 delete mode 100644 synapse/util/debug.py

diff --git a/synapse/config/logger.py b/synapse/config/logger.py
index 63e69a7e0..77ded0ad2 100644
--- a/synapse/config/logger.py
+++ b/synapse/config/logger.py
@@ -22,7 +22,6 @@ import yaml
 from string import Template
 import os
 import signal
-from synapse.util.debug import debug_deferreds
 
 
 DEFAULT_LOG_CONFIG = Template("""
@@ -71,8 +70,6 @@ class LoggingConfig(Config):
         self.verbosity = config.get("verbose", 0)
         self.log_config = self.abspath(config.get("log_config"))
         self.log_file = self.abspath(config.get("log_file"))
-        if config.get("full_twisted_stacktraces"):
-            debug_deferreds()
 
     def default_config(self, config_dir_path, server_name, **kwargs):
         log_file = self.abspath("homeserver.log")
@@ -88,11 +85,6 @@ class LoggingConfig(Config):
 
         # A yaml python logging config file
         log_config: "%(log_config)s"
-
-        # Stop twisted from discarding the stack traces of exceptions in
-        # deferreds by waiting a reactor tick before running a deferred's
-        # callbacks.
-        # full_twisted_stacktraces: true
         """ % locals()
 
     def read_arguments(self, args):
diff --git a/synapse/util/debug.py b/synapse/util/debug.py
deleted file mode 100644
index dc49162e6..000000000
--- a/synapse/util/debug.py
+++ /dev/null
@@ -1,71 +0,0 @@
-# -*- coding: utf-8 -*-
-# Copyright 2015, 2016 OpenMarket Ltd
-#
-# 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.
-
-from twisted.internet import defer, reactor
-from functools import wraps
-from synapse.util.logcontext import LoggingContext, PreserveLoggingContext
-
-
-def debug_deferreds():
-    """Cause all deferreds to wait for a reactor tick before running their
-    callbacks. This increases the chance of getting a stack trace out of
-    a defer.inlineCallback since the code waiting on the deferred will get
-    a chance to add an errback before the deferred runs."""
-
-    # Helper method for retrieving and restoring the current logging context
-    # around a callback.
-    def with_logging_context(fn):
-        context = LoggingContext.current_context()
-
-        def restore_context_callback(x):
-            with PreserveLoggingContext(context):
-                return fn(x)
-
-        return restore_context_callback
-
-    # We are going to modify the __init__ method of defer.Deferred so we
-    # need to get a copy of the old method so we can still call it.
-    old__init__ = defer.Deferred.__init__
-
-    # We need to create a deferred to bounce the callbacks through the reactor
-    # but we don't want to add a callback when we create that deferred so we
-    # we create a new type of deferred that uses the old __init__ method.
-    # This is safe as long as the old __init__ method doesn't invoke an
-    # __init__ using super.
-    class Bouncer(defer.Deferred):
-        __init__ = old__init__
-
-    # We'll add this as a callback to all Deferreds. Twisted will wait until
-    # the bouncer deferred resolves before calling the callbacks of the
-    # original deferred.
-    def bounce_callback(x):
-        bouncer = Bouncer()
-        reactor.callLater(0, with_logging_context(bouncer.callback), x)
-        return bouncer
-
-    # We'll add this as an errback to all Deferreds. Twisted will wait until
-    # the bouncer deferred resolves before calling the errbacks of the
-    # original deferred.
-    def bounce_errback(x):
-        bouncer = Bouncer()
-        reactor.callLater(0, with_logging_context(bouncer.errback), x)
-        return bouncer
-
-    @wraps(old__init__)
-    def new__init__(self, *args, **kargs):
-        old__init__(self, *args, **kargs)
-        self.addCallbacks(bounce_callback, bounce_errback)
-
-    defer.Deferred.__init__ = new__init__