Support abort hooks (symmetrically to commit hooks) #77

CHANGES.rst

@@ -18,6 +18,12 @@
   if it is only an instance of `BaseException` but not of `Exception`,
   such as e.g. a ``SystemExit`` or ``KeyboardInterupt`` exception.
 
+- Support abort hooks (symmetrically to commit hooks)
+  (`#77 <https://github.com/zopefoundation/transaction/issues/77>`_).
+
+- Hooks are now cleared after successfull ``commit`` and ``abort`` to avoid
+  potential cyclic references.
+
 
 2.4.0 (2018-10-23)
 ==================

transaction/_compat.py

@@ -4,23 +4,23 @@
 PY3 = sys.version_info[0] == 3
 JYTHON = sys.platform.startswith('java')
 
-if PY3:
+if PY3:  # pragma: no cover
     text_type = str
 else: # pragma: no cover
     # py2
     text_type = unicode
 
 def bytes_(s, encoding='latin-1', errors='strict'):
-    if isinstance(s, text_type):
+    if isinstance(s, text_type):  # pragma: no cover
         s = s.encode(encoding, errors)
     return s
 
 def text_(s):
-    if not isinstance(s, text_type):
+    if not isinstance(s, text_type):  # pragma: no cover
         s = s.decode('utf-8')
     return s
 
-if PY3:
+if PY3:  # pragma: no cover
     def native_(s, encoding='latin-1', errors='strict'):
         if isinstance(s, text_type):
             return s
@@ -31,7 +31,7 @@ def native_(s, encoding='latin-1', errors='strict'):
             return s.encode(encoding, errors)
         return str(s)
 
-if PY3:
+if PY3:  # pragma: no cover
     from io import StringIO
 else: # pragma: no cover
     from io import BytesIO
@@ -43,7 +43,7 @@ def write(self, s):
             super(StringIO, self).write(s)
 
 
-if PY3:
+if PY3:  # pragma: no cover
     def reraise(tp, value, tb=None):
         if value.__traceback__ is not tb: # pragma: no cover
             raise value.with_traceback(tb)
@@ -67,7 +67,7 @@ def exec_(code, globs=None, locs=None):
 """)
 
 
-try:
+try:  # pragma: no cover
     from threading import get_ident as get_thread_ident
 except ImportError: # pragma: no cover
     # py2

transaction/_transaction.py

@@ -120,6 +120,12 @@ def __init__(self, synchronizers=None, manager=None):
         # List of (hook, args, kws) tuples added by addAfterCommitHook().
         self._after_commit = []
 
+        # List of (hook, args, kws) tuples added by addBeforeAbortHook().
+        self._before_abort = []
+
+        # List of (hook, args, kws) tuples added by addAfterAbortHook().
+        self._after_abort = []
+
     @property
     def _extension(self):
         # for backward compatibility, since most clients used this
@@ -312,9 +318,9 @@ def commit(self):
             finally:
                 del t, v, tb
         else:
-            self._free()
             self._synchronizers.map(lambda s: s.afterCompletion(self))
             self._callAfterCommitHooks(status=True)
+            self._free()
         self.log.debug("commit")
 
     def _saveAndGetCommitishError(self):
@@ -360,12 +366,8 @@ def addBeforeCommitHook(self, hook, args=(), kws=None):
 
     def _callBeforeCommitHooks(self):
         # Call all hooks registered, allowing further registrations
-        # during processing.  Note that calls to addBeforeCommitHook() may
-        # add additional hooks while hooks are running, and iterating over a
-        # growing list is well-defined in Python.
-        for hook, args, kws in self._before_commit:
-            hook(*args, **kws)
-        self._before_commit = []
+        # during processing.
+        self._call_hooks(self._before_commit)
 
     def getAfterCommitHooks(self):
         """ See ITransaction.
@@ -380,34 +382,85 @@ def addAfterCommitHook(self, hook, args=(), kws=None):
         self._after_commit.append((hook, tuple(args), kws))
 
     def _callAfterCommitHooks(self, status=True):
+        self._call_hooks(self._after_commit,
+                         exc=False, clean=True, prefix_args=(status,))
+
+    def _call_hooks(self, hooks, exc=True, clean=False, prefix_args=()):
+        """call *hooks*.
+
+        If *exc* is true, fail on the first exception; otherwise
+        log the exception and continue.
+
+        If *clean* is true, abort all resources. This is to ensure
+        a clean state should a (after) hook has affected one
+        of the resources.
+
+        *prefix_args* defines additioan arguments prefixed
+        to the arguments provided by the hook definition.
+
+        ``_call_hooks`` supports that a hook adds new hooks.
+        """
         # Avoid to abort anything at the end if no hooks are registred.
-        if not self._after_commit:
+        if not hooks:
             return
+        try:
+            # Call all hooks registered, allowing further registrations
+            # during processing
+            for hook, args, kws in hooks:
+                try:
+                    hook(*(prefix_args + args), **kws)
+                except:
+                    if exc:
+                        raise
+                    # We should not fail
+                    self.log.error("Error in hook exec in %s ",
+                                   hook, exc_info=sys.exc_info())
+        finally:
+            del hooks[:]  # clear hooks
+            if not clean:
+                return
+            # The primary operation has already been performed.
+            # But the hooks execution might have left the resources
+            # in an unclean state. Clean up
+            for rm in self._resources:
+                try:
+                    rm.abort(self)
+                except:
+                    # XXX should we take further actions here ?
+                    self.log.error("Error in abort() on manager %s",
+                                   rm, exc_info=sys.exc_info())
+
+    def getBeforeAbortHooks(self):
+        """ See ITransaction.
+        """
+        return iter(self._before_abort)
+
+    def addBeforeAbortHook(self, hook, args=(), kws=None):
+        """ See ITransaction.
+        """
+        if kws is None:
+            kws = {}
+        self._before_abort.append((hook, tuple(args), kws))
+
+    def _callBeforeAbortHooks(self):
         # Call all hooks registered, allowing further registrations
-        # during processing.  Note that calls to addAterCommitHook() may
-        # add additional hooks while hooks are running, and iterating over a
-        # growing list is well-defined in Python.
-        for hook, args, kws in self._after_commit:
-            # The first argument passed to the hook is a Boolean value,
-            # true if the commit succeeded, or false if the commit aborted.
-            try:
-                hook(status, *args, **kws)
-            except:
-                # We need to catch the exceptions if we want all hooks
-                # to be called
-                self.log.error("Error in after commit hook exec in %s ",
-                               hook, exc_info=sys.exc_info())
-        # The transaction is already committed. It must not have
-        # further effects after the commit.
-        for rm in self._resources:
-            try:
-                rm.abort(self)
-            except:
-                # XXX should we take further actions here ?
-                self.log.error("Error in abort() on manager %s",
-                               rm, exc_info=sys.exc_info())
-        self._after_commit = []
-        self._before_commit = []
+        # during processing.
+        self._call_hooks(self._before_abort, exc=False)
+
+    def getAfterAbortHooks(self):
+        """ See ITransaction.
+        """
+        return iter(self._after_abort)
+
+    def addAfterAbortHook(self, hook, args=(), kws=None):
+        """ See ITransaction.
+        """
+        if kws is None:
+            kws = {}
+        self._after_abort.append((hook, tuple(args), kws))
+
+    def _callAfterAbortHooks(self):
+        self._call_hooks(self._after_abort, clean=True)
 
     def _commitResources(self):
         # Execute the two-phase commit protocol.
@@ -469,6 +522,7 @@ def _free(self):
         # to break references---this transaction object will not be returned
         # as the current transaction from its manager after this, and all
         # IDatamanager objects joined to it will forgotten
+        # All hooks are forgotten.
         if self._manager:
             self._manager.free(self)
 
@@ -477,6 +531,11 @@ def _free(self):
 
         del self._resources[:]
 
+        del self._before_commit[:]
+        del self._after_commit[:]
+        del self._before_abort[:]
+        del self._after_abort[:]
+
     def data(self, ob):
         try:
             data = self._data
@@ -499,6 +558,7 @@ def set_data(self, ob, ob_data):
     def abort(self):
         """ See ITransaction.
         """
+        self._callBeforeAbortHooks()
         if self._savepoint2index:
             self._invalidate_all_savepoints()
 
@@ -519,6 +579,7 @@ def abort(self):
                     self.log.error("Failed to abort resource manager: %s",
                                    rm, exc_info=sys.exc_info())
 
+            self._callAfterAbortHooks()
             self._free()
 
             self._synchronizers.map(lambda s: s.afterCompletion(self))

transaction/interfaces.py

@@ -356,6 +356,78 @@ def getAfterCommitHooks():
         by a top-level transaction commit.
         """
 
+    def addBeforeAbortHook(hook, args=(), kws=None):
+        """Register a hook to call before the transaction is abortted.
+
+        The specified hook function will be called after the transaction's
+        abort method has been called, but before the abort process has been
+        started.  The hook will be passed the specified positional (`args`)
+        and keyword (`kws`) arguments.  `args` is a sequence of positional
+        arguments to be passed, defaulting to an empty tuple (no positional
+        arguments are passed).  `kws` is a dictionary of keyword argument
+        names and values to be passed, or the default None (no keyword
+        arguments are passed).
+
+        Multiple hooks can be registered and will be called in the order they
+        were registered (first registered, first called).  This method can
+        also be called from a hook:  an executing hook can register more
+        hooks.  Applications should take care to avoid creating infinite loops
+        by recursively registering hooks.
+
+        Abort hooks are called only for a top-level abort. If the
+        transaction is committed, abort hooks are not called.
+        This is true even if the commit fails. In this case, however,
+        the transaction is in the ``COMMITFAILED`` state and is
+        virtually unusable; therefore, a top-level abort will typically
+        follow.
+
+        Calling a hook "consumes" its registration.
+        Hook registrations do not persist across transactions.
+        """
+
+    def getBeforeAbortHooks():
+        """Return iterable producing the registered addBeforeAbort hooks.
+
+        A triple (hook, args, kws) is produced for each registered hook.
+        The hooks are produced in the order in which they would be invoked
+        by a top-level transaction abort.
+        """
+
+    def addAfterAbortHook(hook, args=(), kws=None):
+        """Register a hook to call after a transaction abort.
+
+        The specified hook function will be called after the transaction
+        abort  with positional arguments `args`  and `kws`
+        keyword arguments.  `args` is a sequence of
+        positional arguments to be passed, defaulting to an empty tuple
+        `kws` is a dictionary of keyword argument names and values to be
+        passed, or the default None (no keyword arguments are passed).
+
+        Multiple hooks can be registered and will be called in the order they
+        were registered (first registered, first called).  This method can
+        also be called from a hook:  an executing hook can register more
+        hooks.  Applications should take care to avoid creating infinite loops
+        by recursively registering hooks.
+
+        Abort hooks are called only for a top-level abort. If the
+        transaction is committed, abort hooks are not called.
+        This is true even if the commit fails. In this case, however,
+        the transaction is in the ``COMMITFAILED`` state and is
+        virtually unusable; therefore, a top-level abort will typically
+        follow.
+
+        Calling a hook "consumes" its registration.
+        Hook registrations do not persist across transactions.
+        """
+
+    def getAfterAbortHooks():
+        """Return iterable producing the registered addAfterAbort hooks.
+
+        A triple (hook, args, kws) is produced for each registered hook.
+        The hooks are produced in the order in which they would be invoked
+        by a top-level transaction abort.
+        """
+
     def set_data(ob, data):
         """Hold data on behalf of an object