Skip to content

Commit 7cc4bfa

Browse files
bzbarsky-applebzbarsky-apple
committed
Add autorelease pools to the Darwin KVS implementation.
KVS can be used from outside the Matter queue in our example apps, and we were leaking things that never got released due to the lack of an autorelease pool around these KVS operations. Many thanks to Vivien Nicolas for catching this.
1 parent e527611 commit 7cc4bfa

File tree

1 file changed

+138
-130
lines changed

1 file changed

+138
-130
lines changed

‎src/platform/Darwin/KeyValueStoreManagerImpl.mm

+138-130
Original file line numberDiff line numberDiff line change
@@ -147,179 +147,187 @@ - (instancetype)initWithContext:(nonnull NSManagedObjectContext *)context key:(n
147147

148148
CHIP_ERROR KeyValueStoreManagerImpl::Init(const char * fileName)
149149
{
150-
if (mInitialized) {
151-
return CHIP_NO_ERROR;
152-
}
150+
@autoreleasepool {
151+
if (mInitialized) {
152+
return CHIP_NO_ERROR;
153+
}
153154

154-
ReturnErrorCodeIf(gContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
155-
ReturnErrorCodeIf(fileName == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
156-
ReturnErrorCodeIf(fileName[0] == '\0', CHIP_ERROR_INVALID_ARGUMENT);
157-
158-
NSURL * url = nullptr;
159-
NSString * filepath = [NSString stringWithUTF8String:fileName];
160-
ReturnErrorCodeIf(filepath == nil, CHIP_ERROR_INVALID_ARGUMENT);
161-
162-
// relative paths are relative to Documents folder
163-
if (![filepath hasPrefix:@"/"]) {
164-
NSURL * documentsDirectory = [NSFileManager.defaultManager URLForDirectory:NSDocumentDirectory
165-
inDomain:NSUserDomainMask
166-
appropriateForURL:nil
167-
create:YES
168-
error:nil];
169-
if (documentsDirectory == nullptr) {
170-
ChipLogError(DeviceLayer, "Failed to get documents directory.");
171-
return CHIP_ERROR_INTERNAL;
155+
ReturnErrorCodeIf(gContext != nullptr, CHIP_ERROR_INCORRECT_STATE);
156+
ReturnErrorCodeIf(fileName == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
157+
ReturnErrorCodeIf(fileName[0] == '\0', CHIP_ERROR_INVALID_ARGUMENT);
158+
159+
NSURL * url = nullptr;
160+
NSString * filepath = [NSString stringWithUTF8String:fileName];
161+
ReturnErrorCodeIf(filepath == nil, CHIP_ERROR_INVALID_ARGUMENT);
162+
163+
// relative paths are relative to Documents folder
164+
if (![filepath hasPrefix:@"/"]) {
165+
NSURL * documentsDirectory = [NSFileManager.defaultManager URLForDirectory:NSDocumentDirectory
166+
inDomain:NSUserDomainMask
167+
appropriateForURL:nil
168+
create:YES
169+
error:nil];
170+
if (documentsDirectory == nullptr) {
171+
ChipLogError(DeviceLayer, "Failed to get documents directory.");
172+
return CHIP_ERROR_INTERNAL;
173+
}
174+
ChipLogProgress(
175+
DeviceLayer, "Found user documents directory: %s", [[documentsDirectory absoluteString] UTF8String]);
176+
177+
url = [NSURL URLWithString:filepath relativeToURL:documentsDirectory];
178+
} else {
179+
url = [NSURL fileURLWithPath:filepath];
172180
}
173-
ChipLogProgress(
174-
DeviceLayer, "Found user documents directory: %s", [[documentsDirectory absoluteString] UTF8String]);
181+
ReturnErrorCodeIf(url == nullptr, CHIP_ERROR_NO_MEMORY);
175182

176-
url = [NSURL URLWithString:filepath relativeToURL:documentsDirectory];
177-
} else {
178-
url = [NSURL fileURLWithPath:filepath];
179-
}
180-
ReturnErrorCodeIf(url == nullptr, CHIP_ERROR_NO_MEMORY);
183+
ChipLogProgress(DeviceLayer, "KVS will be written to: %s", [[url absoluteString] UTF8String]);
181184

182-
ChipLogProgress(DeviceLayer, "KVS will be written to: %s", [[url absoluteString] UTF8String]);
185+
NSManagedObjectModel * model = CreateManagedObjectModel();
186+
ReturnErrorCodeIf(model == nullptr, CHIP_ERROR_NO_MEMORY);
183187

184-
NSManagedObjectModel * model = CreateManagedObjectModel();
185-
ReturnErrorCodeIf(model == nullptr, CHIP_ERROR_NO_MEMORY);
188+
// setup persistent store coordinator
186189

187-
// setup persistent store coordinator
190+
NSPersistentStoreCoordinator * coordinator = [[NSPersistentStoreCoordinator alloc] initWithManagedObjectModel:model];
188191

189-
NSPersistentStoreCoordinator * coordinator = [[NSPersistentStoreCoordinator alloc] initWithManagedObjectModel:model];
192+
NSError * error = nil;
193+
if (![coordinator addPersistentStoreWithType:NSSQLiteStoreType configuration:nil URL:url options:nil error:&error]) {
194+
ChipLogError(DeviceLayer, "Invalid store. Attempting to clear: %s", error.localizedDescription.UTF8String);
195+
if (![[NSFileManager defaultManager] removeItemAtURL:url error:&error]) {
196+
ChipLogError(DeviceLayer, "Failed to delete item: %s", error.localizedDescription.UTF8String);
197+
}
190198

191-
NSError * error = nil;
192-
if (![coordinator addPersistentStoreWithType:NSSQLiteStoreType configuration:nil URL:url options:nil error:&error]) {
193-
ChipLogError(DeviceLayer, "Invalid store. Attempting to clear: %s", error.localizedDescription.UTF8String);
194-
if (![[NSFileManager defaultManager] removeItemAtURL:url error:&error]) {
195-
ChipLogError(DeviceLayer, "Failed to delete item: %s", error.localizedDescription.UTF8String);
199+
if (![coordinator addPersistentStoreWithType:NSSQLiteStoreType
200+
configuration:nil
201+
URL:url
202+
options:nil
203+
error:&error]) {
204+
ChipLogError(DeviceLayer, "Failed to initialize clear KVS storage: %s", error.localizedDescription.UTF8String);
205+
chipDie();
206+
}
196207
}
197208

198-
if (![coordinator addPersistentStoreWithType:NSSQLiteStoreType
199-
configuration:nil
200-
URL:url
201-
options:nil
202-
error:&error]) {
203-
ChipLogError(DeviceLayer, "Failed to initialize clear KVS storage: %s", error.localizedDescription.UTF8String);
204-
chipDie();
205-
}
206-
}
209+
// create Managed Object context
210+
gContext = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
211+
[gContext setMergePolicy:NSMergeByPropertyObjectTrumpMergePolicy];
212+
[gContext setPersistentStoreCoordinator:coordinator];
207213

208-
// create Managed Object context
209-
gContext = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
210-
[gContext setMergePolicy:NSMergeByPropertyObjectTrumpMergePolicy];
211-
[gContext setPersistentStoreCoordinator:coordinator];
212-
213-
mInitialized = true;
214-
return CHIP_NO_ERROR;
214+
mInitialized = true;
215+
return CHIP_NO_ERROR;
216+
} // @autoreleasepool
215217
}
216218

217219
CHIP_ERROR KeyValueStoreManagerImpl::_Get(
218220
const char * key, void * value, size_t value_size, size_t * read_bytes_size, size_t offset)
219221
{
220-
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
221-
ReturnErrorCodeIf(offset != 0, CHIP_ERROR_INVALID_ARGUMENT);
222-
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
223-
224-
KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil, true);
225-
if (!item) {
226-
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
227-
}
222+
@autoreleasepool {
223+
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
224+
ReturnErrorCodeIf(offset != 0, CHIP_ERROR_INVALID_ARGUMENT);
225+
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
226+
227+
KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil, true);
228+
if (!item) {
229+
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
230+
}
228231

229-
__block NSData * itemValue = nil;
230-
// can only access this object on the managed queue
231-
[gContext performBlockAndWait:^{
232-
itemValue = item.value;
233-
}];
232+
__block NSData * itemValue = nil;
233+
// can only access this object on the managed queue
234+
[gContext performBlockAndWait:^{
235+
itemValue = item.value;
236+
}];
234237

235-
if (read_bytes_size != nullptr) {
236-
*read_bytes_size = itemValue.length;
237-
}
238+
if (read_bytes_size != nullptr) {
239+
*read_bytes_size = itemValue.length;
240+
}
238241

239-
if (value != nullptr) {
240-
memcpy(value, itemValue.bytes, std::min<size_t>((itemValue.length), value_size));
242+
if (value != nullptr) {
243+
memcpy(value, itemValue.bytes, std::min<size_t>((itemValue.length), value_size));
241244
#if CHIP_CONFIG_DARWIN_STORAGE_VERBOSE_LOGGING
242-
fprintf(stderr, "GETTING VALUE FOR: '%s': ", key);
243-
for (size_t i = 0; i < std::min<size_t>((itemValue.length), value_size); ++i) {
244-
fprintf(stderr, "%02x ", static_cast<uint8_t *>(value)[i]);
245-
}
246-
fprintf(stderr, "\n");
245+
fprintf(stderr, "GETTING VALUE FOR: '%s': ", key);
246+
for (size_t i = 0; i < std::min<size_t>((itemValue.length), value_size); ++i) {
247+
fprintf(stderr, "%02x ", static_cast<uint8_t *>(value)[i]);
248+
}
249+
fprintf(stderr, "\n");
247250
#endif
248-
}
251+
}
249252

250-
if (itemValue.length > value_size) {
251-
return CHIP_ERROR_BUFFER_TOO_SMALL;
252-
}
253+
if (itemValue.length > value_size) {
254+
return CHIP_ERROR_BUFFER_TOO_SMALL;
255+
}
253256

254-
return CHIP_NO_ERROR;
257+
return CHIP_NO_ERROR;
258+
} // @autoreleasepool
255259
}
256260

257261
CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key)
258262
{
259-
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
260-
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
263+
@autoreleasepool {
264+
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
265+
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
261266

262-
KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil);
263-
if (!item) {
264-
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
265-
}
267+
KeyValueItem * item = FindItemForKey([[NSString alloc] initWithUTF8String:key], nil);
268+
if (!item) {
269+
return CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
270+
}
266271

267-
__block BOOL success = NO;
268-
__block NSError * error = nil;
269-
[gContext performBlockAndWait:^{
270-
[gContext deleteObject:item];
271-
success = [gContext save:&error];
272-
}];
272+
__block BOOL success = NO;
273+
__block NSError * error = nil;
274+
[gContext performBlockAndWait:^{
275+
[gContext deleteObject:item];
276+
success = [gContext save:&error];
277+
}];
273278

274-
if (!success) {
275-
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
276-
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
277-
}
279+
if (!success) {
280+
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
281+
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
282+
}
278283

279-
return CHIP_NO_ERROR;
284+
return CHIP_NO_ERROR;
285+
} // @autoreleasepool
280286
}
281287

282288
CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, size_t value_size)
283289
{
284-
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
285-
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
286-
287-
NSData * data = [[NSData alloc] initWithBytes:value length:value_size];
288-
289-
NSString * itemKey = [[NSString alloc] initWithUTF8String:key];
290-
ReturnErrorCodeIf(itemKey == nil, CHIP_ERROR_INVALID_ARGUMENT);
290+
@autoreleasepool {
291+
ReturnErrorCodeIf(key == nullptr, CHIP_ERROR_INVALID_ARGUMENT);
292+
ReturnErrorCodeIf(gContext == nullptr, CHIP_ERROR_UNINITIALIZED);
293+
294+
NSData * data = [[NSData alloc] initWithBytes:value length:value_size];
295+
296+
NSString * itemKey = [[NSString alloc] initWithUTF8String:key];
297+
ReturnErrorCodeIf(itemKey == nil, CHIP_ERROR_INVALID_ARGUMENT);
298+
299+
KeyValueItem * item = FindItemForKey(itemKey, nil);
300+
if (!item) {
301+
[gContext performBlockAndWait:^{
302+
[gContext insertObject:[[KeyValueItem alloc] initWithContext:gContext key:itemKey value:data]];
303+
}];
304+
} else {
305+
[gContext performBlockAndWait:^{
306+
item.value = data;
307+
}];
308+
}
291309

292-
KeyValueItem * item = FindItemForKey(itemKey, nil);
293-
if (!item) {
310+
__block BOOL success = NO;
311+
__block NSError * error = nil;
294312
[gContext performBlockAndWait:^{
295-
[gContext insertObject:[[KeyValueItem alloc] initWithContext:gContext key:itemKey value:data]];
313+
success = [gContext save:&error];
296314
}];
297-
} else {
298-
[gContext performBlockAndWait:^{
299-
item.value = data;
300-
}];
301-
}
302315

303-
__block BOOL success = NO;
304-
__block NSError * error = nil;
305-
[gContext performBlockAndWait:^{
306-
success = [gContext save:&error];
307-
}];
308-
309-
if (!success) {
310-
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
311-
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
312-
}
316+
if (!success) {
317+
ChipLogError(DeviceLayer, "Error saving context: %s", error.localizedDescription.UTF8String);
318+
return CHIP_ERROR_PERSISTED_STORAGE_FAILED;
319+
}
313320

314321
#if CHIP_CONFIG_DARWIN_STORAGE_VERBOSE_LOGGING
315-
fprintf(stderr, "PUT VALUE FOR: '%s': ", key);
316-
for (size_t i = 0; i < value_size; ++i) {
317-
fprintf(stderr, "%02x ", static_cast<const uint8_t *>(value)[i]);
318-
}
319-
fprintf(stderr, "\n");
322+
fprintf(stderr, "PUT VALUE FOR: '%s': ", key);
323+
for (size_t i = 0; i < value_size; ++i) {
324+
fprintf(stderr, "%02x ", static_cast<const uint8_t *>(value)[i]);
325+
}
326+
fprintf(stderr, "\n");
320327
#endif
321328

322-
return CHIP_NO_ERROR;
329+
return CHIP_NO_ERROR;
330+
} // @autoreleasepool
323331
}
324332

325333
} // namespace PersistedStorage

0 commit comments

Comments
 (0)