Skip to content

Commit 3e8aeeb

Browse files
[app] Fix DeferredAttributePersister memory leak (project-chip#31075)
* [app] Fix DeferredAttributePerister memory leak ScopedMemoryBuffer's Release() method was used instead of Free(). Add CHECK_RETURN_VALUE annotation to the Release() method to prevent from making such a mistake in the future. Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no> * Code review --------- Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
1 parent ca577f4 commit 3e8aeeb

File tree

3 files changed

+24
-12
lines changed

3 files changed

+24
-12
lines changed

src/app/DeferredAttributePersistenceProvider.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void DeferredAttribute::Flush(AttributePersistenceProvider & persister)
3939
{
4040
VerifyOrReturn(IsArmed());
4141
persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize()));
42-
mValue.Release();
42+
mValue.Free();
4343
}
4444

4545
CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue)

src/darwin/Framework/CHIP/MTRBaseDevice.mm

+2-2
Original file line numberDiff line numberDiff line change
@@ -1142,8 +1142,8 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
11421142
//
11431143
callback->AdoptReadClient(std::move(readClient));
11441144
callback.release();
1145-
attributePathParamsList.Release();
1146-
eventPathParamsList.Release();
1145+
IgnoreUnusedVariable(attributePathParamsList.Release());
1146+
IgnoreUnusedVariable(eventPathParamsList.Release());
11471147
return err;
11481148
});
11491149
std::move(*bridge).DispatchAction(self);

src/lib/support/ScopedBuffer.h

+21-9
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#pragma once
2626

2727
#include <lib/support/CHIPMem.h>
28+
#include <lib/support/CodeUtils.h>
2829

2930
#include <type_traits>
3031
#include <utility>
@@ -84,10 +85,11 @@ class ScopedMemoryBufferBase
8485
const void * Ptr() const { return mBuffer; }
8586

8687
/**
87-
* Releases the undelying buffer. Buffer stops being managed and will not be
88-
* auto-freed.
88+
* Releases the underlying buffer.
89+
*
90+
* The buffer stops being managed and will not be auto-freed.
8991
*/
90-
void * Release()
92+
CHECK_RETURN_VALUE void * Release()
9193
{
9294
void * buffer = mBuffer;
9395
mBuffer = nullptr;
@@ -139,13 +141,18 @@ class ScopedMemoryBuffer : public Impl::ScopedMemoryBufferBase<MemoryManagement>
139141

140142
static_assert(std::is_trivially_destructible<T>::value, "Destructors won't get run");
141143

142-
inline T * Get() { return static_cast<T *>(Base::Ptr()); }
143-
inline T & operator[](size_t index) { return Get()[index]; }
144+
T * Get() { return static_cast<T *>(Base::Ptr()); }
145+
T & operator[](size_t index) { return Get()[index]; }
144146

145-
inline const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
146-
inline const T & operator[](size_t index) const { return Get()[index]; }
147+
const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
148+
const T & operator[](size_t index) const { return Get()[index]; }
147149

148-
inline T * Release() { return static_cast<T *>(Base::Release()); }
150+
/**
151+
* Releases the underlying buffer.
152+
*
153+
* The buffer stops being managed and will not be auto-freed.
154+
*/
155+
CHECK_RETURN_VALUE T * Release() { return static_cast<T *>(Base::Release()); }
149156

150157
ScopedMemoryBuffer & Calloc(size_t elementCount)
151158
{
@@ -222,7 +229,12 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
222229
ScopedMemoryBuffer<T>::Free();
223230
}
224231

225-
T * Release()
232+
/**
233+
* Releases the underlying buffer.
234+
*
235+
* The buffer stops being managed and will not be auto-freed.
236+
*/
237+
CHECK_RETURN_VALUE T * Release()
226238
{
227239
T * buffer = ScopedMemoryBuffer<T>::Release();
228240
mCount = 0;

0 commit comments

Comments
 (0)