[ZCM] [ZC] 789/10 Comment "Zope's transaction behaviour flawed"

[Collector: Zope Bugs, Features, and Patches ...]

Issue #789 Update (Comment) "Zope's transaction behaviour flawed"
 Status Pending, Zope/bug critical
To followup, visit:
  http://collector.zope.org/Zope/789

==============================================================
= Comment - Entry #10 by tim_one on Aug 24, 2004 8:04 pm

As explained in this thread:

http://mail.zope.org/pipermail/zodb-dev/2004-August/007865.html

ZODB isn't helping matters here.  The short course is that when a connection is closed without an explicit commit() or abort(), and objects from that connection have pending modifications, things are a mess -- and especially if the pending changes are due to subtransaction commits.  The next ZODB 3.2 release will raise an exception if a connection is closed with changes pending (better dead than insane <0.5 wink>).
________________________________________
= Comment - Entry #9 by tim_one on Aug 24, 2004 8:00 pm

As explained in this thread:

http://mail.zope.org/pipermail/zodb-dev/2004-August/007865.html

________________________________________
= Comment - Entry #8 by dunny on Aug 24, 2004 6:52 pm

See link for patch to publisher that may help:

http://mail.zope.org/pipermail/zope-dev/attachments/20040825/e5d7a8e9/publisher-abort.obj

I'm currently running some tests for sessions and transience. And this patch seems to iron out a lot problems that have been reported with transience of recent.

________________________________________
= Resubmit - Entry #7 by efge on Aug 24, 2004 8:47 am

 Status: Resolved => Pending

I'm reopening because the current fix (and Zope 2.7.2) poses problems. It seems in some cases of error the transaction is not aborted correctly and some objects linger in the cache.

The following external method will help reproduce the problem:

def install(self):
    myfolder = self.myfolder
    nbfold = len(myfolder.objectIds())
    id = "glop"+str(nbfold)
    myfolder.manage_addFolder(id)
    get_transaction().commit(1)
    # The raise should cancel all object creation
    raise Exception(id)

Create a folder 'myfolder', and run the method several times. You'll get the creation of several objects. If however you purge the ZODB cache ("minimize" button), or if for some other reason the cache is cleaned, or another thread is used, you'll see that the objects disappear.

Sometimes we get POSKeyError because the created objects are used by another transaction to create a reference, and later the original object disappears.

The core of the problem seems to be that in Publish.py in the case of an error, the transaction is not always aborted. Reverting to 2.7.0, or just re-adding a
        if transactions_manager:
            transactions_manager.abort()
like there was after the except in 2.7.0 (which I know is the whole point of this bug) makes the problem go away.

________________________________________
= Comment - Entry #6 by mcdonc on May 23, 2004 2:01 am

Historical record:  See also issue 869 for more discussion/detail.
________________________________________
= Resolve - Entry #5 by mcdonc on May 23, 2004 1:57 am

 Status: Pending => Resolved

A fix for this has been checked in for 2.7.1+ and on the HEAD.  I am resolving, although comments on the patch are welcome and encouraged.
________________________________________
= Comment - Entry #4 by d.maurer on Apr 29, 2004 6:43 pm

Toby convinced me ("zope-dev" discussion) that the correct behaviour would be to abort the transaction after error handling
(thus doing error handling in the same transaction as the request).

Currently, at the start of error handling, "get_transaction().begin()" is called, effectivle aborting the transaction, the request has been running in. This is bad as people expecting to find information in the SESSION are mislead.

I do not understand why implementing the correct behaviour would break application that "clear ... data
themselves at transaction boundaries".

I can see that application will break that perform a commit in the error handling -- but that should be rare...
________________________________________
= Comment - Entry #3 by mcdonc on Apr 29, 2004 12:09 pm

Michael Dunstan also compiled some notes and fixes for this... see http://mail.zope.org/pipermail/zope-dev/2004-April/022727.html
________________________________________
= Comment - Entry #2 by stevea on Feb 2, 2003 12:14 pm

I'm currently implementing similar behaviour for zope 3.

However, introducing this behaviour in zope2 could break applications that themselves clear cached or computed data on transaction boundaries. This data would not be available for the error page.

I think ZPatterns applications sometimes rely on the current behaviour, so this change would break certain zpatterns applications.
________________________________________
= Request - Entry #1 by d.maurer on Feb 2, 2003 10:39 am

Zope's current transaction behaviour is essentially:


  ## request starts
  transaction.begin()
  try:
       object= REQUEST.traverse(...)
       mapply(object,...)
       transaction.commit()
  except:
       transaction.abort()
       handle_error()
  ## request ends


This is flawed as error handling is done outside of a transaction.

   Potential changes during the error handling spill over
   uncontrolled into another request and are there
   either committed or aborted as part of this request.

   Andrew Athan (<mailto:aathan at memeplex.com>) has had lots
   of serious inconsistencies in Zope's session data.
   After extensive analysis, he found out that reading
   the session data during error handling led to these
   error conditions (reading session data causes writes to
   the administrative data).


I suggest, we let Zope perform error handling in its own
transaction after the original transaction had been aborted.
When error handling succeeds, its transaction is committed,
otherwise aborted.

The new behaviour would look something like this:

  ## request starts
  transaction.begin()
  try:
       object= REQUEST.traverse(...)
       mapply(object,...)
       transaction.commit()
  except:
       transaction.abort()
       transaction.begin()
       transaction.note('%s (application error handling)'
                        % '/'.join(object.getPhysicalPath)
			)
       try:
	   handle_error()
	   transaction.commit()
       except:
           default_handle_error() # Zope's default error handling
	                          # it should not have side effects
				  # and is executed inside the
				  # error handling transaction
	   transaction.abort()
  ## request ends


--- See discussion in "zope-dev".
==============================================================