http://bugzilla.zeroc.com/bugzilla/show_bug.cgi?id=2233

Squashed commit of the following:

commit 9c5b39640659633dfad70341c4e709eeda8d5f8e
Author: Matthew Newhook <matthew@zeroc.com>
Date:   Thu Sep 6 18:13:59 2007 +0800

    More fixes.

commit 49cade00fee4dfc00fd688ba1ca8bcbf80376b59
Author: Matthew Newhook <matthew@zeroc.com>
Date:   Thu Sep 6 17:07:36 2007 +0800

    removed debug.

commit f33af74a2fe64ee2433228133c58afe6278fc5f3
Author: Matthew Newhook <matthew@zeroc.com>
Date:   Thu Sep 6 17:03:01 2007 +0800

    fixes.

commit a145684f88b000087a6fe5551bb9018ad03c8307
Author: Matthew Newhook <matthew@zeroc.com>
Date:   Thu Sep 6 15:25:37 2007 +0800

    altered reference counting as suggested in the bug report.

1 files changed, 34 insertions, 33 deletions

diff --git a/cpp/src/Freeze/TransactionI.cpp b/cpp/src/Freeze/TransactionI.cpp
index f7844748ed8..d4e82e0d221 100644
--- a/cpp/src/Freeze/TransactionI.cpp
+++ b/cpp/src/Freeze/TransactionI.cpp
@@ -87,6 +87,22 @@ Freeze::TransactionI::commit()
 void
 Freeze::TransactionI::rollback()
 {
+    rollbackInternal(false);
+}
+
+Freeze::ConnectionPtr
+Freeze::TransactionI::getConnection() const
+{
+    if(_txn)
+    {
+        return _connection;
+    }
+    return 0;
+}
+
+void
+Freeze::TransactionI::rollbackInternal(bool warning)
+{
     if(_txn != 0)
     {
         long txnId = 0;
@@ -94,9 +110,15 @@ Freeze::TransactionI::rollback()
         {
             _connection->closeAllIterators();
             
-            if(_txTrace >= 1)
+            if(_txTrace >= 1 || (warning && _warnRollback))
             {
                 txnId = (_txn->id() & 0x7FFFFFFF) + 0x80000000L;
+                if(warning && _warnRollback)
+                {
+                    Warning warn(_communicator->getLogger());
+                    warn << "Freeze.Transaction: rolled back transaction " << hex << txnId << dec
+                         << " due to destruction.\nApplication code should explicitely call rollback or commit.";
+                }
             }
             
             _txn->abort();
@@ -139,32 +161,17 @@ Freeze::TransactionI::rollback()
     }
 }
 
-Freeze::ConnectionPtr
-Freeze::TransactionI::getConnection() const
-{
-    return _connection;
-}
-
 //
 // External refcount operations, from code holding a Transaction[I]Ptr
 //
 void
 Freeze::TransactionI::__decRef()
 {
-    // If dropping the second to last reference and there is still a
-    // transaction then this means the last reference is held by the
-    // connection. In this case we must rollback the transaction.
-    bool rb = false;
-    if(__getRef() == 2 && _txn)
+    if(__getRef() == 2 && _txn && _connection->__getRef() == 1)
     {
-        rb = true;
+        rollbackInternal(true);
     }
     Shared::__decRef();
-    if(rb)
-    {
-        rollback();
-        // After this the transaction is dead.
-    }
 }
 
 void
@@ -173,10 +180,17 @@ Freeze::TransactionI::setPostCompletionCallback(const Freeze::PostCompletionCall
     _postCompletionCallback = cb;
 }
     
-Freeze::TransactionI::TransactionI(const ConnectionIPtr& connection) :
+//
+// The constructor takes a ConnectionI* instead of a ConnectionIPtr
+// because we have to ensure there is no call to __decRef while the
+// transaction or the connection are not assigned to a Ptr in
+// user-code.
+//
+Freeze::TransactionI::TransactionI(ConnectionI* connection) :
     _communicator(connection->communicator()),
     _connection(connection),
     _txTrace(connection->txTrace()),
+    _warnRollback(_communicator->getProperties()->getPropertyAsIntWithDefault("Freeze.Warn.Rollback", 1)),
     _txn(0)
 {
     try
@@ -235,19 +249,6 @@ Freeze::TransactionI::postCompletion(bool committed, bool deadlock)
         _postCompletionCallback->postCompletion(committed, deadlock);
     }
 
-    // Its necessary here to copy the connection before calling
-    // clearTransaction because this may release the last reference. This specifically
-    // occurs in the following scenario:
-    //
-    // TransactionalEvictorContext holds the _tx. It calls
-    // _tx->commit(). This comes into this method, and calls
-    // _postCompletionCallback. This causes the context to drop the
-    // _tx reference (reference count is now 1). The
-    // connection->clearTransaction() is then called which drops its
-    // reference causing the transaction to be deleted.
-    //
-    ConnectionIPtr con = _connection;
-    _connection = 0; // Drop the connection
-    con->clearTransaction();
+    _connection->clearTransaction();
     // At this point the transaction may be dead.
 }