From 55249d4723dc624b6ad2aca79bcb8827bcddb17d Mon Sep 17 00:00:00 2001 From: Mike Bruin Date: Tue, 4 Feb 2025 08:59:45 -0500 Subject: [PATCH 1/3] NR-364636 used weak reference to prevent retain cycles and strong references to ensure the object is not deallocated while being used within the block. --- Agent/Analytics/NRMAAnalytics.mm | 2 +- Agent/Analytics/NRMAEventManager.m | 2 +- Agent/Analytics/PersistentEventStore.m | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Agent/Analytics/NRMAAnalytics.mm b/Agent/Analytics/NRMAAnalytics.mm index 16fa2c4f..52945748 100644 --- a/Agent/Analytics/NRMAAnalytics.mm +++ b/Agent/Analytics/NRMAAnalytics.mm @@ -115,7 +115,7 @@ - (id) initWithSessionStartTimeMS:(long long) sessionStartTime { PersistentEventStore *eventStore = [[PersistentEventStore alloc] initWithFilename:filename andMinimumDelay:.025]; - _eventManager = [[NRMAEventManager alloc] initWithPersistentStore:eventStore]; + _eventManager = [[NRMAEventManager alloc] initWithPersistentStore:[eventStore autorelease]]; _attributeValidator = [[NRMAAttributeValidator alloc] init]; _sessionAttributeManager = [[NRMASAM alloc] initWithAttributeValidator:_attributeValidator]; diff --git a/Agent/Analytics/NRMAEventManager.m b/Agent/Analytics/NRMAEventManager.m index 24fc6300..a8d74365 100644 --- a/Agent/Analytics/NRMAEventManager.m +++ b/Agent/Analytics/NRMAEventManager.m @@ -77,7 +77,7 @@ - (BOOL)didReachMaxQueueTime:(NSTimeInterval)currentTimeMilliseconds { } NSTimeInterval oldestEventAge = currentTimeMilliseconds - oldestEventTimestamp; - return (oldestEventAge / 1000) + kBufferTimeSecondsLeeway >= maxBufferTimeSeconds; + return (oldestEventAge / kDefaultBufferSize) + kBufferTimeSecondsLeeway >= maxBufferTimeSeconds; } - (NSUInteger)getEvictionIndex { diff --git a/Agent/Analytics/PersistentEventStore.m b/Agent/Analytics/PersistentEventStore.m index f2a331fe..a388533d 100644 --- a/Agent/Analytics/PersistentEventStore.m +++ b/Agent/Analytics/PersistentEventStore.m @@ -41,14 +41,24 @@ - (nonnull instancetype)initWithFilename:(NSString *)filename } - (void)performWrite:(void (^)(void))writeBlock { + __weak PersistentEventStore *weakSelf = self; dispatch_async(self.writeQueue, ^{ - if (self.pendingBlock != nil) { - dispatch_block_cancel(self.pendingBlock); + __strong PersistentEventStore *strongSelf = weakSelf; + if (!strongSelf) return; // Ensure strongSelf is not nil + + if (strongSelf.pendingBlock != nil) { + dispatch_block_cancel(strongSelf.pendingBlock); } - self.pendingBlock = dispatch_block_create(0, writeBlock); + strongSelf.pendingBlock = dispatch_block_create(0, writeBlock); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(self->_minimumDelay * NSEC_PER_SEC)), self->_writeQueue, self.pendingBlock); + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(strongSelf->_minimumDelay * NSEC_PER_SEC)), strongSelf->_writeQueue, ^{ + __strong PersistentEventStore *innerStrongSelf = weakSelf; + if (innerStrongSelf && innerStrongSelf.pendingBlock) { + innerStrongSelf.pendingBlock(); + innerStrongSelf.pendingBlock = nil; // Release the block after execution + } + }); }); } From 6aca4e4e2521cab058e8736a7ac6dad32edc0dab Mon Sep 17 00:00:00 2001 From: Mike Bruin Date: Tue, 4 Feb 2025 13:42:32 -0500 Subject: [PATCH 2/3] Added a log when we try to run a block but was deallocated, added the pendingblock to cancel in dealloc also --- Agent/Analytics/PersistentEventStore.m | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Agent/Analytics/PersistentEventStore.m b/Agent/Analytics/PersistentEventStore.m index a388533d..02a36938 100644 --- a/Agent/Analytics/PersistentEventStore.m +++ b/Agent/Analytics/PersistentEventStore.m @@ -40,11 +40,20 @@ - (nonnull instancetype)initWithFilename:(NSString *)filename return self; } +- (void) dealloc { + if(self.pendingBlock){ + dispatch_block_cancel(self.pendingBlock); + } +} + - (void)performWrite:(void (^)(void))writeBlock { __weak PersistentEventStore *weakSelf = self; dispatch_async(self.writeQueue, ^{ __strong PersistentEventStore *strongSelf = weakSelf; - if (!strongSelf) return; // Ensure strongSelf is not nil + if (!strongSelf) { // Ensure strongSelf is not nil + NRLOG_WARNING(@"A block was scheduled but PersistentEventStore was deallocated before running"); + return; + } if (strongSelf.pendingBlock != nil) { dispatch_block_cancel(strongSelf.pendingBlock); From 6beaf72868a42c1170b3de4781c1aa3fd4c0a720 Mon Sep 17 00:00:00 2001 From: Mike Bruin Date: Wed, 5 Feb 2025 09:39:40 -0500 Subject: [PATCH 3/3] Changed to use NRLOG_AGENT_WARNING --- Agent/Analytics/PersistentEventStore.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Agent/Analytics/PersistentEventStore.m b/Agent/Analytics/PersistentEventStore.m index 02a36938..e4916728 100644 --- a/Agent/Analytics/PersistentEventStore.m +++ b/Agent/Analytics/PersistentEventStore.m @@ -51,7 +51,7 @@ - (void)performWrite:(void (^)(void))writeBlock { dispatch_async(self.writeQueue, ^{ __strong PersistentEventStore *strongSelf = weakSelf; if (!strongSelf) { // Ensure strongSelf is not nil - NRLOG_WARNING(@"A block was scheduled but PersistentEventStore was deallocated before running"); + NRLOG_AGENT_WARNING(@"A block was scheduled but PersistentEventStore was deallocated before running"); return; }