Refcounting is now thread-safe

4 files changed, 106 insertions, 28 deletions

diff --git a/cpp/src/Freeze/ConnectionI.cpp b/cpp/src/Freeze/ConnectionI.cpp
index dae584e3bbd..258060f9f5c 100644
--- a/cpp/src/Freeze/ConnectionI.cpp
+++ b/cpp/src/Freeze/ConnectionI.cpp
@@ -77,19 +77,44 @@ Freeze::ConnectionI::getName() const
     return _envName;
 }
 
-//
-// External refcount operations, from code holding a Connection[I]Ptr
-//
+void
+Freeze::ConnectionI::__incRef()
+{
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    _refCount++;
+}
+
+
 void
 Freeze::ConnectionI::__decRef()
 {
-    if(__getRef() == 2 && _transaction && _transaction->__getRef() == 1)
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    if(--_refCount == 0)
+    {
+        sync.release();
+        delete this;
+    }
+    else if(_refCount == 1 && _transaction != 0 && _transaction->dbTxn() != 0 && _transaction->__getRefNoSync() == 1)
     {
+        sync.release();
         close();
     }
-    Shared::__decRef();
 }
 
+int
+Freeze::ConnectionI::__getRef() const
+{
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    return _refCount;
+}
+
+int
+Freeze::ConnectionI::__getRefNoSync() const
+{
+    return _refCount;
+}
+
+
 Freeze::ConnectionI::~ConnectionI()
 {
     close();
@@ -101,7 +126,9 @@ Freeze::ConnectionI::ConnectionI(const SharedDbEnvPtr& dbEnv) :
     _envName(dbEnv->getEnvName()),
     _trace(_communicator->getProperties()->getPropertyAsInt("Freeze.Trace.Map")),
     _txTrace(_communicator->getProperties()->getPropertyAsInt("Freeze.Trace.Transaction")),
-    _deadlockWarning(_communicator->getProperties()->getPropertyAsInt("Freeze.Warn.Deadlocks") != 0)
+    _deadlockWarning(_communicator->getProperties()->getPropertyAsInt("Freeze.Warn.Deadlocks") != 0),
+    _refCountMutex(new SharedMutex),
+    _refCount(0)
 {
 }
 
diff --git a/cpp/src/Freeze/ConnectionI.h b/cpp/src/Freeze/ConnectionI.h
index 31d8e206eb5..38b85aa9d2b 100644
--- a/cpp/src/Freeze/ConnectionI.h
+++ b/cpp/src/Freeze/ConnectionI.h
@@ -21,6 +21,17 @@ namespace Freeze
 
 class MapHelperI;
 
+//
+// A mutex shared by a connection and all its transactions
+// (for refcounting thread-safety)
+//
+struct SharedMutex : public IceUtil::Shared
+{
+    IceUtil::Mutex mutex;
+};
+typedef IceUtil::Handle<SharedMutex> SharedMutexPtr;
+
+
 class ConnectionI : public Connection
 {
 public:
@@ -38,8 +49,10 @@ public:
     //
     // Custom refcounting implementation
     //
+    virtual void __incRef();
     virtual void __decRef();
-
+    virtual int __getRef() const;
+ 
     virtual ~ConnectionI();
 
     ConnectionI(const SharedDbEnvPtr&);
@@ -70,6 +83,10 @@ public:
 
 private:
 
+    friend class TransactionI;
+
+    int __getRefNoSync() const;
+
     const Ice::CommunicatorPtr _communicator;
     SharedDbEnvPtr _dbEnv;
     const std::string _envName;
@@ -78,6 +95,8 @@ private:
     const Ice::Int _trace;
     const Ice::Int _txTrace;
     const bool _deadlockWarning;
+    SharedMutexPtr _refCountMutex;
+    int _refCount;
 };  
 typedef IceUtil::Handle<ConnectionI> ConnectionIPtr;
 
diff --git a/cpp/src/Freeze/TransactionI.cpp b/cpp/src/Freeze/TransactionI.cpp
index 082a495c21d..a11c0eb44c4 100644
--- a/cpp/src/Freeze/TransactionI.cpp
+++ b/cpp/src/Freeze/TransactionI.cpp
@@ -161,17 +161,41 @@ Freeze::TransactionI::rollbackInternal(bool warning)
     }
 }
 
-//
-// External refcount operations, from code holding a Transaction[I]Ptr
-//
+void
+Freeze::TransactionI::__incRef()
+{
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    _refCount++;
+}
+
+
 void
 Freeze::TransactionI::__decRef()
 {
-    if(__getRef() == 2 && _txn && _connection->__getRef() == 1)
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    if(--_refCount == 0)
     {
+        sync.release();
+        delete this;
+    }
+    else if(_txn != 0 && _refCount == 1 && _connection->__getRefNoSync() == 1)
+    {
+        sync.release();
         rollbackInternal(true);
     }
-    Shared::__decRef();
+}
+
+int
+Freeze::TransactionI::__getRef() const
+{
+    IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+    return _refCount;
+}
+
+int
+Freeze::TransactionI::__getRefNoSync() const
+{
+    return _refCount;
 }
 
 void
@@ -191,7 +215,9 @@ Freeze::TransactionI::TransactionI(ConnectionI* connection) :
     _connection(connection),
     _txTrace(connection->txTrace()),
     _warnRollback(_communicator->getProperties()->getPropertyAsIntWithDefault("Freeze.Warn.Rollback", 1)),
-    _txn(0)
+    _txn(0),
+    _refCountMutex(connection->_refCountMutex),
+    _refCount(0)
 {
     try
     {
@@ -221,18 +247,7 @@ Freeze::TransactionI::TransactionI(ConnectionI* connection) :
     
 Freeze::TransactionI::~TransactionI()
 {
-    if(_txn != 0)
-    {
-        try
-        {
-            rollback();
-        }
-        catch(const IceUtil::Exception& e)
-        {
-            Error error(_communicator->getLogger());
-            error << "transaction rollback raised :" << e;
-        }
-    }
+    assert(_txn == 0);
 }
 
 void
@@ -242,13 +257,21 @@ Freeze::TransactionI::postCompletion(bool committed, bool deadlock)
     // calling both the post completion callback and
     // Connection::clearTransaction may alter the transaction
     // reference count which checks _txn.
-    _txn = 0;
+
+    {
+        //
+        // We synchronize here as _txn is checked (read) in the refcounting code
+        //
+        IceUtil::Mutex::Lock sync(_refCountMutex->mutex);
+        _txn = 0;
+    }
 
     if(_postCompletionCallback != 0)
     {
         _postCompletionCallback->postCompletion(committed, deadlock);
     }
 
-    _connection->clearTransaction();
-    // At this point the transaction may be dead.
+    ConnectionIPtr connection = _connection;
+    _connection = 0;
+    connection->clearTransaction(); // may release the last _refCount
 }
diff --git a/cpp/src/Freeze/TransactionI.h b/cpp/src/Freeze/TransactionI.h
index cb5da8a4e61..0172e5126c5 100644
--- a/cpp/src/Freeze/TransactionI.h
+++ b/cpp/src/Freeze/TransactionI.h
@@ -20,6 +20,9 @@ namespace Freeze
 class ConnectionI;
 typedef IceUtil::Handle<ConnectionI> ConnectionIPtr;
 
+class SharedMutex;
+typedef IceUtil::Handle<SharedMutex> SharedMutexPtr;
+
 class PostCompletionCallback : public virtual IceUtil::Shared
 {
 public:
@@ -42,7 +45,9 @@ public:
     //
     // Custom refcounting implementation
     //
+    virtual void __incRef();
     virtual void __decRef();
+    virtual int __getRef() const;
 
     void rollbackInternal(bool);
     void setPostCompletionCallback(const PostCompletionCallbackPtr&);
@@ -60,6 +65,8 @@ private:
 
     friend class ConnectionI;
     
+    int __getRefNoSync() const;
+  
     void postCompletion(bool, bool);
 
     const Ice::CommunicatorPtr _communicator;
@@ -68,6 +75,8 @@ private:
     const Ice::Int _warnRollback;
     DbTxn* _txn;
     PostCompletionCallbackPtr _postCompletionCallback;
+    SharedMutexPtr _refCountMutex;
+    int _refCount;
 };
 
 typedef IceUtil::Handle<TransactionI> TransactionIPtr;