Factor run_in_background out from preserve_fn

It annoys me that we create temporary function objects when there's really no
need for it. Let's factor the gubbins out of preserve_fn and start using it.

docs/log_contexts.rst

@@ -279,9 +279,9 @@ Obviously that option means that the operations done in
 that might be fixed by setting a different logcontext via a ``with
 LoggingContext(...)`` in ``background_operation``).
 
-The second option is to use ``logcontext.preserve_fn``, which wraps a function
+The second option is to use ``logcontext.run_in_background``, which wraps a
-so that it doesn't reset the logcontext even when it returns an incomplete
+function so that it doesn't reset the logcontext even when it returns an
-deferred, and adds a callback to the returned deferred to reset the
+incomplete deferred, and adds a callback to the returned deferred to reset the
 logcontext. In other words, it turns a function that follows the Synapse rules
 about logcontexts and Deferreds into one which behaves more like an external
 function — the opposite operation to that described in the previous section.
@@ -293,7 +293,7 @@ It can be used like this:
     def do_request_handling():
         yield foreground_operation()
 
-        logcontext.preserve_fn(background_operation)()
+        logcontext.run_in_background(background_operation)
 
         # this will now be logged against the request context
         logger.debug("Request handling complete")

synapse/util/logcontext.py

@@ -292,36 +292,41 @@ class PreserveLoggingContext(object):
 
 
 def preserve_fn(f):
-    """Wraps a function, to ensure that the current context is restored after
+    """Function decorator which wraps the function with run_in_background"""
+    def g(*args, **kwargs):
+        return run_in_background(f, *args, **kwargs)
+    return g
+
+
+def run_in_background(f, *args, **kwargs):
+    """Calls a function, ensuring that the current context is restored after
     return from the function, and that the sentinel context is set once the
     deferred returned by the funtion completes.
 
     Useful for wrapping functions that return a deferred which you don't yield
     on.
     """
-    def g(*args, **kwargs):
+    current = LoggingContext.current_context()
-        current = LoggingContext.current_context()
+    res = f(*args, **kwargs)
-        res = f(*args, **kwargs)
+    if isinstance(res, defer.Deferred) and not res.called:
-        if isinstance(res, defer.Deferred) and not res.called:
+        # The function will have reset the context before returning, so
-            # The function will have reset the context before returning, so
+        # we need to restore it now.
-            # we need to restore it now.
+        LoggingContext.set_current_context(current)
-            LoggingContext.set_current_context(current)
 
-            # The original context will be restored when the deferred
+        # The original context will be restored when the deferred
-            # completes, but there is nothing waiting for it, so it will
+        # completes, but there is nothing waiting for it, so it will
-            # get leaked into the reactor or some other function which
+        # get leaked into the reactor or some other function which
-            # wasn't expecting it. We therefore need to reset the context
+        # wasn't expecting it. We therefore need to reset the context
-            # here.
+        # here.
-            #
+        #
-            # (If this feels asymmetric, consider it this way: we are
+        # (If this feels asymmetric, consider it this way: we are
-            # effectively forking a new thread of execution. We are
+        # effectively forking a new thread of execution. We are
-            # probably currently within a ``with LoggingContext()`` block,
+        # probably currently within a ``with LoggingContext()`` block,
-            # which is supposed to have a single entry and exit point. But
+        # which is supposed to have a single entry and exit point. But
-            # by spawning off another deferred, we are effectively
+        # by spawning off another deferred, we are effectively
-            # adding a new exit point.)
+        # adding a new exit point.)
-            res.addBoth(_set_context_cb, LoggingContext.sentinel)
+        res.addBoth(_set_context_cb, LoggingContext.sentinel)
-        return res
+    return res
-    return g
 
 
 def make_deferred_yieldable(deferred):