Remove refcounting (reland of #3356) (#3405)

* Remove refcounting (reland of #3356) This needs a re-think; there are several problems currently: - refcounting of objects with static storage duration - refcounting of objects with automatic storage duration (stack objects) - refcounting of subobjects - including multiple base class subobjects (diamond inheritance) The majority of current uses are in contexts where lifetime could not be extended by reference counting and in general, the initial reference is not dropped. The remaining case looks like a case of unique ownership. If delegate interfaces really need to be refcounted, we'll need to ban multiple inheritance of them (without resorting to virtual inheritance). Reland with some #include needed after #3397. fixes #3348 * Add missing #include <CHIPMem.h> * Add more missing includes
project-chip · Oct 26, 2020 · 5d99466 · 5d99466
1 parent 0b4445a
commit 5d99466
Show file tree

Hide file tree

Showing 11 changed files with 16 additions and 72 deletions.
diff --git a/examples/chip-tool/commands/common/Commands.cpp b/examples/chip-tool/commands/common/Commands.cpp
@@ -24,6 +24,8 @@
 #include <algorithm>
 #include <string>
 
+#include <support/CHIPMem.h>
+
 void Commands::Register(const char * clusterName, commands_list commandsList)
 {
     for (auto & command : commandsList)

diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp
@@ -30,6 +30,7 @@
 #include <controller/CHIPDeviceController.h>
 #include <jni.h>
 #include <pthread.h>
+#include <support/CHIPMem.h>
 #include <support/CodeUtils.h>
 #include <support/ErrorStr.h>
 #include <support/logging/CHIPLogging.h>

diff --git a/src/transport/NetworkProvisioning.cpp b/src/transport/NetworkProvisioning.cpp
@@ -32,24 +32,9 @@ namespace chip {
 
 void NetworkProvisioning::Init(NetworkProvisioningDelegate * delegate, DeviceNetworkProvisioningDelegate * deviceDelegate)
 {
-    if (mDelegate != nullptr)
-    {
-        mDelegate->Release();
-    }
-
-    if (delegate != nullptr)
-    {
-        mDelegate = delegate->Retain();
-    }
-
-    if (mDeviceDelegate != nullptr)
-    {
-        mDeviceDelegate->Release();
-    }
-
     if (deviceDelegate != nullptr)
     {
-        mDeviceDelegate = deviceDelegate->Retain();
+        mDeviceDelegate = deviceDelegate;
 #if CONFIG_DEVICE_LAYER
         DeviceLayer::PlatformMgr().AddEventHandler(ConnectivityHandler, reinterpret_cast<intptr_t>(this));
 #endif
@@ -60,17 +45,10 @@ NetworkProvisioning::~NetworkProvisioning()
 {
     if (mDeviceDelegate != nullptr)
     {
-        mDeviceDelegate->Release();
-
 #if CONFIG_DEVICE_LAYER
         DeviceLayer::PlatformMgr().RemoveEventHandler(ConnectivityHandler, reinterpret_cast<intptr_t>(this));
 #endif
     }
-
-    if (mDelegate != nullptr)
-    {
-        mDelegate->Release();
-    }
 }
 
 CHIP_ERROR NetworkProvisioning::HandleNetworkProvisioningMessage(uint8_t msgType, System::PacketBuffer * msgBuf)

diff --git a/src/transport/NetworkProvisioning.h b/src/transport/NetworkProvisioning.h
@@ -24,7 +24,6 @@
 #pragma once
 
 #include <core/CHIPCore.h>
-#include <core/ReferenceCounted.h>
 #include <protocols/CHIPProtocols.h>
 #include <support/BufBound.h>
 #include <system/SystemPacketBuffer.h>
@@ -36,7 +35,7 @@
 
 namespace chip {
 
-class DLL_EXPORT DeviceNetworkProvisioningDelegate : public ReferenceCounted<DeviceNetworkProvisioningDelegate>
+class DLL_EXPORT DeviceNetworkProvisioningDelegate
 {
 public:
     /**
@@ -51,7 +50,7 @@ class DLL_EXPORT DeviceNetworkProvisioningDelegate : public ReferenceCounted<Dev
     virtual ~DeviceNetworkProvisioningDelegate() {}
 };
 
-class DLL_EXPORT NetworkProvisioningDelegate : public ReferenceCounted<NetworkProvisioningDelegate>
+class DLL_EXPORT NetworkProvisioningDelegate
 {
 public:
     /**

diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp
@@ -52,8 +52,6 @@ CHIP_ERROR RendezvousSession::Init(const RendezvousParameters & params)
         Transport::BLE * transport = chip::Platform::New<Transport::BLE>();
         err                        = transport->Init(this, mParams);
         mTransport                 = transport;
-        mTransport->Retain();
-        transport->Release();
     }
 #endif // CONFIG_NETWORK_LAYER_BLE
     SuccessOrExit(err);
@@ -74,7 +72,7 @@ RendezvousSession::~RendezvousSession()
 {
     if (mTransport)
     {
-        mTransport->Release();
+        chip::Platform::Delete(mTransport);
         mTransport = nullptr;
     }
 

diff --git a/src/transport/SecurePairingSession.cpp b/src/transport/SecurePairingSession.cpp
@@ -49,10 +49,6 @@ SecurePairingSession::SecurePairingSession() {}
 
 SecurePairingSession::~SecurePairingSession()
 {
-    if (mDelegate != nullptr)
-    {
-        mDelegate->Release();
-    }
     memset(&mPoint[0], 0, sizeof(mPoint));
     memset(&mWS[0][0], 0, sizeof(mWS));
     memset(&mKe[0], 0, sizeof(mKe));
@@ -146,11 +142,7 @@ CHIP_ERROR SecurePairingSession::Init(uint32_t setupCode, uint32_t pbkdf2IterCou
                         sizeof(mWS), &mWS[0][0]);
     SuccessOrExit(err);
 
-    if (mDelegate != nullptr)
-    {
-        mDelegate->Release();
-    }
-    mDelegate    = delegate->Retain();
+    mDelegate    = delegate;
     mLocalNodeId = myNodeId;
     mLocalKeyId  = myKeyId;
 

diff --git a/src/transport/SecurePairingSession.h b/src/transport/SecurePairingSession.h
@@ -26,7 +26,6 @@
 
 #pragma once
 
-#include <core/ReferenceCounted.h>
 #include <crypto/CHIPCryptoPAL.h>
 #include <support/Base64.h>
 #include <system/SystemPacketBuffer.h>
@@ -39,7 +38,7 @@ extern const char * kSpake2pR2ISessionInfo;
 
 using namespace Crypto;
 
-class DLL_EXPORT SecurePairingSessionDelegate : public ReferenceCounted<SecurePairingSessionDelegate>
+class DLL_EXPORT SecurePairingSessionDelegate
 {
 public:
     /**

diff --git a/src/transport/SecureSessionMgr.cpp b/src/transport/SecureSessionMgr.cpp
@@ -51,33 +51,17 @@ SecureSessionMgrBase::SecureSessionMgrBase() : mState(State::kNotReady) {}
 SecureSessionMgrBase::~SecureSessionMgrBase()
 {
     CancelExpiryTimer();
-
-    if (mCB != nullptr)
-    {
-        mCB->Release();
-    }
-
-    if (mTransport)
-    {
-        mTransport->Release();
-    }
 }
 
 CHIP_ERROR SecureSessionMgrBase::InitInternal(NodeId localNodeId, System::Layer * systemLayer, Transport::Base * transport)
 {
-    if (mTransport)
-    {
-        mTransport->Release();
-        mTransport = nullptr;
-    }
-
     CHIP_ERROR err = CHIP_NO_ERROR;
     VerifyOrExit(mState == State::kNotReady, err = CHIP_ERROR_INCORRECT_STATE);
 
     mState       = State::kInitialized;
     mLocalNodeId = localNodeId;
     mSystemLayer = systemLayer;
-    mTransport   = transport->Retain();
+    mTransport   = transport;
 
     mTransport->SetMessageReceiveHandler(HandleDataReceived, this);
     mPeerConnections.SetConnectionExpiredHandler(HandleConnectionExpired, this);

diff --git a/src/transport/SecureSessionMgr.h b/src/transport/SecureSessionMgr.h
@@ -28,7 +28,6 @@
 #include <utility>
 
 #include <core/CHIPCore.h>
-#include <core/ReferenceCounted.h>
 #include <inet/IPAddress.h>
 #include <inet/IPEndPointBasis.h>
 #include <support/CodeUtils.h>
@@ -50,7 +49,7 @@ class SecureSessionMgrBase;
  *   is interested in receiving these callbacks, they can specialize this class and handle
  *   each trigger in their implementation of this class.
  */
-class DLL_EXPORT SecureSessionMgrDelegate : public ReferenceCounted<SecureSessionMgrDelegate>
+class DLL_EXPORT SecureSessionMgrDelegate
 {
 public:
     /**
@@ -91,7 +90,7 @@ class DLL_EXPORT SecureSessionMgrDelegate : public ReferenceCounted<SecureSessio
     virtual ~SecureSessionMgrDelegate() {}
 };
 
-class DLL_EXPORT SecureSessionMgrBase : public ReferenceCounted<SecureSessionMgrBase>
+class DLL_EXPORT SecureSessionMgrBase
 {
 public:
     /**
@@ -114,14 +113,7 @@ class DLL_EXPORT SecureSessionMgrBase : public ReferenceCounted<SecureSessionMgr
      * @details
      *   Release if there was an existing callback object
      */
-    void SetDelegate(SecureSessionMgrDelegate * cb)
-    {
-        if (mCB != nullptr)
-        {
-            mCB->Release();
-        }
-        mCB = cb->Retain();
-    }
+    void SetDelegate(SecureSessionMgrDelegate * cb) { mCB = cb; }
 
     /**
      * @brief

diff --git a/src/transport/raw/Base.h b/src/transport/raw/Base.h
@@ -24,7 +24,6 @@
 #pragma once
 
 #include <core/CHIPError.h>
-#include <core/ReferenceCounted.h>
 #include <inet/IPAddress.h>
 #include <inet/UDPEndPoint.h>
 #include <system/SystemPacketBuffer.h>
@@ -39,7 +38,7 @@ namespace Transport {
  * packing by encoding and decoding headers) and generic message transport
  * methods.
  */
-class Base : public ReferenceCounted<Base>
+class Base
 {
 public:
     virtual ~Base() {}

diff --git a/src/transport/tests/TestSecurePairingSession.cpp b/src/transport/tests/TestSecurePairingSession.cpp
@@ -28,11 +28,11 @@
 
 #include <core/CHIPCore.h>
 #include <core/CHIPSafeCasts.h>
-#include <transport/SecurePairingSession.h>
-
 #include <stdarg.h>
+#include <support/CHIPMem.h>
 #include <support/CodeUtils.h>
 #include <support/TestUtils.h>
+#include <transport/SecurePairingSession.h>
 
 using namespace chip;