[ZODB-Dev] Re: [Zope-dev] Re: post publishing hook

[Tim Peters]

...
>> The most obvious failure is that the hook gets called on both commit()
>> and abort().  I view that as a mistake in the design of
>> beforeCompletion; there are currently no uses of beforeCompletion()
>> anywhere in ZODB, so I doubt it would cause any pain to change that.
>> Perhaps the hook method could be passed a string arg, 'commit' or
>> 'abort'.

[Florent Guillaume]
> Actually it can check txn.status to decide.

Are you sure?  I don't see it.  .status isn't changed until after
beforeCompletion() is called:

    def commit(self, subtransaction=False):
        if self.status is Status.COMMITFAILED:
            self._prior_commit_failed() # doesn't return

        if not subtransaction and self._sub and self._resources:
            # This commit is for a top-level transaction that has
            # previously committed subtransactions.  Do one last
            # subtransaction commit to clear out the current objects,
            # then commit all the subjars.
            self.commit(True)

        if not subtransaction:
            for s in self._synchronizers:
                s.beforeCompletion(self)
            self.status = Status.COMMITTING

in 3.3, or in 3.4 the tail end is:

        if not subtransaction:
            self._synchronizers.map(lambda s: s.beforeCompletion(self))
            self.status = Status.COMMITTING

.status isn't changed before calling beforeCompletion() in the abort() case
either.

>> A subtler failure is that "are called for subsequent transactions until
>> they are unregistered" isn't the full truth:  the TM holds only a weak
>> reference to a registered synchronizer.  If such a synchronizer becomes
>> otherwise unreferenced, it will vanish from the TM's weak set of
>> registered synchronizers, and then its methods won't get invoked at all.
>> If the caller arranges to hold a strong reference to a OneShot instance
>> until the current transaction ends, then that part isn't an issue.

> Why the weak refs? What does it help doing in the ZODB case?

I'm not really sure -- gotta ask Jim.  He said it was important to cater to
people who open Connection objects but never close them.  And, indeed, the
Zope3 test suite sucked up enormous amounts of RAM precisely because some
tests did exactly that, before all this synch stuff in transaction managers
got rewritten to use "weak sets" of synchronizers instead of a list of
synchronizers (and in the latter case, registering with a TM made a
synchronizer effectively immortal).

The change above between 3.3 and 3.4 is highly relevant here too.  It turned
out that materializing a list of the still-living elements of a weak set (in
order to iterate over it) was unacceptable, because doing so has a very high
chance of tricking Python's garbage collector into moving those elements
into older generations, with the end result that synchronizers were staying
alive for a very long time after all strong references went away.  Weak sets
grew their own .map() method then, materializing only one weakly-referenced
thing at a time.  There's more explanation in ZODB/utils.py WeakSet.map in
the ZODB 3.4 branch.