NR-364636 fixing the persistent store memory leak (#345)

* 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. * Added a log when we try to run a block but was deallocated, added the pendingblock to cancel in dealloc also * Changed to use NRLOG_AGENT_WARNING
newrelic · Feb 5, 2025 · d18debd · d18debd
1 parent 472e2a8
commit d18debd
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 6 deletions.
diff --git 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
@@ -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
@@ -40,15 +40,34 @@ - (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, ^{
-        if (self.pendingBlock != nil) {
-            dispatch_block_cancel(self.pendingBlock);
+        __strong PersistentEventStore *strongSelf = weakSelf;
+        if (!strongSelf) { // Ensure strongSelf is not nil
+            NRLOG_AGENT_WARNING(@"A block was scheduled but PersistentEventStore was deallocated before running");
+            return;
         }
 
-        self.pendingBlock = dispatch_block_create(0, writeBlock);
+        if (strongSelf.pendingBlock != nil) {
+            dispatch_block_cancel(strongSelf.pendingBlock);
+        }
+
+        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
+            }
+        });
     });
 }