[PLAT-6799] Improve metadata performance with async file I/O

Bugsnag/BugsnagSystemState.h

@@ -9,13 +9,15 @@
 #import <Foundation/Foundation.h>
 
 #import "BugsnagConfiguration.h"
+#import "BugsnagKeys.h"
 
 #define SYSTEMSTATE_KEY_APP @"app"
 #define SYSTEMSTATE_KEY_DEVICE @"device"
 
 #define SYSTEMSTATE_APP_WAS_TERMINATED @"wasTerminated"
 #define SYSTEMSTATE_APP_IS_ACTIVE @"isActive"
 #define SYSTEMSTATE_APP_IS_IN_FOREGROUND @"inForeground"
+#define SYSTEMSTATE_APP_IS_LAUNCHING BSGKeyIsLaunching
 #define SYSTEMSTATE_APP_VERSION @"version"
 #define SYSTEMSTATE_APP_BUNDLE_VERSION @"bundleVersion"
 #define SYSTEMSTATE_APP_DEBUGGER_IS_ACTIVE @"debuggerIsActive"
@@ -38,6 +40,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic) NSUInteger consecutiveLaunchCrashes;
 
+- (void)markLaunchCompleted;
+
 /**
  * Purge all stored system state.
  */

Bugsnag/BugsnagSystemState.m

@@ -25,7 +25,6 @@
 #import "BSG_KSSystemInfo.h"
 #import "BSG_RFC3339DateTool.h"
 #import "BugsnagKVStoreObjC.h"
-#import "BugsnagKeys.h"
 #import "BugsnagLogger.h"
 #import "BugsnagSessionTracker.h"
 #import "BugsnagSystemState.h"
@@ -57,10 +56,16 @@
     NSMutableDictionary *app = state[SYSTEMSTATE_KEY_APP];
 
     // KV-store versions of these are authoritative
-    app[SYSTEMSTATE_APP_WAS_TERMINATED] = [kvstore NSBooleanForKey:SYSTEMSTATE_APP_WAS_TERMINATED defaultValue:false];
-    app[SYSTEMSTATE_APP_IS_ACTIVE] = [kvstore NSBooleanForKey:SYSTEMSTATE_APP_IS_ACTIVE defaultValue:false];
-    app[SYSTEMSTATE_APP_IS_IN_FOREGROUND] = [kvstore NSBooleanForKey:SYSTEMSTATE_APP_IS_IN_FOREGROUND defaultValue:false];
-    app[SYSTEMSTATE_APP_DEBUGGER_IS_ACTIVE] = [kvstore NSBooleanForKey:SYSTEMSTATE_APP_DEBUGGER_IS_ACTIVE defaultValue:false];
+    for (NSString *key in @[SYSTEMSTATE_APP_DEBUGGER_IS_ACTIVE,
+                            SYSTEMSTATE_APP_IS_ACTIVE,
+                            SYSTEMSTATE_APP_IS_IN_FOREGROUND,
+                            SYSTEMSTATE_APP_IS_LAUNCHING,
+                            SYSTEMSTATE_APP_WAS_TERMINATED]) {
+        NSNumber *value = [kvstore NSBooleanForKey:key defaultValue:nil];
+        if (value != nil) {
+            app[key] = value;
+        }
+    }
 
     return state;
 }
@@ -90,6 +95,7 @@ id blankIfNil(id value) {
     [kvstore deleteKey:SYSTEMSTATE_APP_WAS_TERMINATED];
     [kvstore setBoolean:isActive forKey:SYSTEMSTATE_APP_IS_ACTIVE];
     [kvstore setBoolean:isInForeground forKey:SYSTEMSTATE_APP_IS_IN_FOREGROUND];
+    [kvstore setBoolean:true forKey:SYSTEMSTATE_APP_IS_LAUNCHING];
     [kvstore setBoolean:isBeingDebugged forKey:SYSTEMSTATE_APP_DEBUGGER_IS_ACTIVE];
 
     NSMutableDictionary *app = [NSMutableDictionary new];
@@ -248,6 +254,10 @@ - (void)setConsecutiveLaunchCrashes:(NSUInteger)consecutiveLaunchCrashes {
     [self setValue:@(_consecutiveLaunchCrashes = consecutiveLaunchCrashes) forKey:ConsecutiveLaunchCrashesKey inSection:InternalKey];
 }
 
+- (void)markLaunchCompleted {
+    [self.kvStore setBoolean:false forKey:SYSTEMSTATE_APP_IS_LAUNCHING];
+}
+
 - (void)setValue:(id)value forAppKey:(NSString *)key {
     [self setValue:value forKey:key inSection:SYSTEMSTATE_KEY_APP];
 }

Bugsnag/Client/BugsnagClient.m

@@ -32,6 +32,7 @@
 #import "BSGConnectivity.h"
 #import "BSGEventUploader.h"
 #import "BSGFileLocations.h"
+#import "BSGGlobals.h"
 #import "BSGInternalErrorReporter.h"
 #import "BSGJSONSerialization.h"
 #import "BSGNotificationBreadcrumbs.h"
@@ -92,12 +93,12 @@
     // Contains the state of the event (handled/unhandled)
     char *handledState;
     // Contains the user-specified metadata, including the user tab from config.
-    char *metadataPath;
+    char *metadataJSON;
     // Contains the Bugsnag configuration, all under the "config" tab.
     char *configPath;
-    // Contains notifier state, under "deviceState" and crash-specific
+    // Contains notifier state under "deviceState", and crash-specific
     // information under "crash".
-    char *statePath;
+    char *stateJSON;
     // User onCrash handler
     void (*onCrash)(const BSG_KSCrashReportWriter *writer);
 } bsg_g_bugsnag_data;
@@ -128,8 +129,8 @@ void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer, __attrib
     }
     if (isCrash) {
         writer->addJSONFileElement(writer, "config", bsg_g_bugsnag_data.configPath);
-        writer->addJSONFileElement(writer, "metaData", bsg_g_bugsnag_data.metadataPath);
-        writer->addJSONFileElement(writer, "state", bsg_g_bugsnag_data.statePath);
+        writer->addJSONElement(writer, "metaData", bsg_g_bugsnag_data.metadataJSON);
+        writer->addJSONElement(writer, "state", bsg_g_bugsnag_data.stateJSON);
         BugsnagBreadcrumbsWriteCrashReport(writer);
         if (watchdogSentinelPath != NULL) {
             // Delete the file to indicate a handled termination
@@ -249,11 +250,9 @@ - (instancetype)initWithConfiguration:(BugsnagConfiguration *)configuration {
         _configMetadataFromLastLaunch = [BSGJSONSerialization JSONObjectWithContentsOfFile:_configMetadataFile options:0 error:nil];
 
         _metadataFile = fileLocations.metadata;
-        bsg_g_bugsnag_data.metadataPath = strdup(_metadataFile.fileSystemRepresentation);
         _metadataFromLastLaunch = [BSGJSONSerialization JSONObjectWithContentsOfFile:_metadataFile options:0 error:nil];
 
         _stateMetadataFile = fileLocations.state;
-        bsg_g_bugsnag_data.statePath = strdup(_stateMetadataFile.fileSystemRepresentation);
         _stateMetadataFromLastLaunch = [BSGJSONSerialization JSONObjectWithContentsOfFile:_stateMetadataFile options:0 error:nil];
 
         self.stateEventBlocks = [NSMutableArray new];
@@ -284,20 +283,8 @@ - (instancetype)initWithConfiguration:(BugsnagConfiguration *)configuration {
         _lastOrientation = BSGOrientationNameFromEnum([UIDEVICE currentDevice].orientation);
         [self.state addMetadata:_lastOrientation withKey:BSGKeyOrientation toSection:BSGKeyDeviceState];
 #endif
-        // sync initial state
-        [self metadataChanged:self.metadata];
-        [self metadataChanged:self.state];
-
-        // add observers for future metadata changes
-        // weakSelf is used as the BugsnagClient will always be instantiated
-        // for the entire lifecycle of an application, and there is therefore
-        // no need to check for strong self
-        __weak __typeof__(self) weakSelf = self;
-        void (^observer)(BugsnagStateEvent *) = ^(BugsnagStateEvent *event) {
-            [weakSelf metadataChanged:event.data];
-        };
-        [self.metadata addObserverWithBlock:observer];
-        [self.state addObserverWithBlock:observer];
+        [self.metadata setStorageBuffer:&bsg_g_bugsnag_data.metadataJSON file:_metadataFile];
+        [self.state setStorageBuffer:&bsg_g_bugsnag_data.stateJSON file:_stateMetadataFile];
 
         self.pluginClient = [[BugsnagPluginClient alloc] initWithPlugins:self.configuration.plugins
                                                                   client:self];
@@ -443,6 +430,7 @@ - (void)markLaunchCompleted {
     bsg_log_debug(@"App has finished launching");
     [self.appLaunchTimer invalidate];
     [self.state addMetadata:@NO withKey:BSGKeyIsLaunching toSection:BSGKeyApp];
+    [self.systemState markLaunchCompleted];
 }
 
 - (void)sendLaunchCrashSynchronously {
@@ -575,8 +563,14 @@ - (void)computeDidCrashLastLaunch {
 
     self.appDidCrashLastLaunch = didCrash;
 
-    BOOL wasLaunching = [self.stateMetadataFromLastLaunch[BSGKeyApp][BSGKeyIsLaunching] boolValue];
-    BOOL didCrashDuringLaunch = didCrash && wasLaunching;
+    NSNumber *wasLaunching = ({
+        // BugsnagSystemState's KV-store is now the reliable source of the isLaunching status.
+        self.systemState.lastLaunchState[SYSTEMSTATE_KEY_APP][SYSTEMSTATE_APP_IS_LAUNCHING] ?:
+        // Earlier notifier versions stored it only in state.json - but due to async I/O this is no longer accurate.
+        self.stateMetadataFromLastLaunch[BSGKeyApp][BSGKeyIsLaunching];
+    });
+
+    BOOL didCrashDuringLaunch = didCrash && wasLaunching.boolValue;
     if (didCrashDuringLaunch) {
         self.systemState.consecutiveLaunchCrashes++;
     } else {
@@ -964,16 +958,6 @@ - (void)addBreadcrumbWithBlock:(void (^)(BugsnagBreadcrumb *))block {
     [self.breadcrumbs addBreadcrumbWithBlock:block];
 }
 
-- (void)metadataChanged:(BugsnagMetadata *)metadata {
-    @synchronized(metadata) {
-        if (metadata == self.metadata) {
-            [BSGJSONSerialization writeJSONObject:[metadata toDictionary] toFile:self.metadataFile options:0 error:nil];
-        } else if (metadata == self.state) {
-            [BSGJSONSerialization writeJSONObject:[metadata toDictionary] toFile:self.stateMetadataFile options:0 error:nil];
-        }
-    }
-}
-
 /**
  * Update the device status in response to a battery change notification
  *

Bugsnag/Helpers/BugsnagKVStoreObjC.h

@@ -29,7 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (bool)booleanForKey:(NSString*)key defaultValue:(bool)defaultValue;
 
-- (NSNumber*)NSBooleanForKey:(NSString*)key defaultValue:(bool)defaultValue;
+- (nullable NSNumber*)NSBooleanForKey:(NSString*)key defaultValue:(nullable NSNumber*)defaultValue;
 
 - (void)setString:(NSString*)value forKey:(NSString*)key;
 

Bugsnag/Helpers/BugsnagKVStoreObjC.m

@@ -63,8 +63,13 @@ - (bool)booleanForKey:(NSString*)key defaultValue:(bool)defaultValue {
     return value;
 }
 
-- (NSNumber*)NSBooleanForKey:(NSString*)key defaultValue:(bool)defaultValue {
-    return [NSNumber numberWithBool:[self booleanForKey:key defaultValue:defaultValue]];
+- (nullable NSNumber*)NSBooleanForKey:(NSString*)key defaultValue:(nullable NSNumber*)defaultValue {
+    int err = 0;
+    bool boolValue = bsgkv_getBoolean([key UTF8String], &err);
+    if (err != 0) {
+        return defaultValue;
+    }
+    return [NSNumber numberWithBool:boolValue];
 }
 
 - (void)setString:(NSString*)value forKey:(NSString*)key {

Bugsnag/Metadata/BugsnagMetadata.m

@@ -26,14 +26,26 @@
 
 #import "BugsnagMetadata+Private.h"
 
+#import "BSGGlobals.h"
+#import "BSGJSONSerialization.h"
 #import "BSGSerialization.h"
 #import "BugsnagLogger.h"
 #import "BugsnagStateEvent.h"
 
+
 @interface BugsnagMetadata ()
+
 @property(atomic, readwrite, strong) NSMutableArray *stateEventBlocks;
+
+@property (assign, nonatomic) char **buffer;
+@property (copy, nonatomic) NSString *file;
+@property (copy, nonatomic) NSData *pendingWrite;
+
 @end
 
+
+// MARK: -
+
 @implementation BugsnagMetadata
 
 - (instancetype)init {
@@ -91,7 +103,9 @@ - (NSMutableArray *)sanitizeArray:(NSArray *)obj {
 }
 
 - (NSDictionary *)toDictionary {
-    return [self.dictionary mutableCopy];
+    @synchronized (self) {
+        return [self.dictionary mutableCopy];
+    }
 }
 
 - (void)notifyObservers {
@@ -190,7 +204,7 @@ - (void)addMetadata:(NSDictionary *)metadataValues
         }
         if (![oldValue isEqual:metadata]) {
             self.dictionary[sectionName] = metadata.count ? metadata : nil;
-            [self notifyObservers];
+            [self didChangeValue];
         }
     }
 }
@@ -214,17 +228,93 @@ - (void)clearMetadataFromSection:(NSString *)sectionName
 {
     @synchronized(self) {
         [self.dictionary removeObjectForKey:sectionName];
+        [self didChangeValue];
     }
-    [self notifyObservers];
 }
 
 - (void)clearMetadataFromSection:(NSString *)section
                          withKey:(NSString *)key
 {
     @synchronized(self) {
         [(NSMutableDictionary *)self.dictionary[section] removeObjectForKey:key];
+        [self didChangeValue];
+    }
+}
+
+// MARK: -
+
+- (void)didChangeValue {
+    if (self.buffer || self.file) {
+        [self serialize];
     }
     [self notifyObservers];
 }
 
+- (void)setStorageBuffer:(char * _Nullable *)buffer file:(NSString *)file {
+    self.buffer = buffer;
+    self.file = file;
+    [self serialize];
+}
+
+- (void)serialize {
+    NSError *error = nil;
+    NSData *data = [BSGJSONSerialization dataWithJSONObject:[self toDictionary] options:0 error:&error];
+    if (!data) {
+        bsg_log_err(@"%s: %@", __FUNCTION__, error);
+        return;
+    }
+    if (self.buffer) {
+        [self writeData:data toBuffer:self.buffer];
+    }
+    if (self.file) {
+        [self writeData:data toFile:self.file];
+    }
+}
+
+//
+// Metadata is stored in memory as a JSON encoded C string so that it is accessible at crash time.
+//
+- (void)writeData:(NSData *)data toBuffer:(char **)buffer {
+    char *newbuffer = calloc(1, data.length + 1);
+    if (!newbuffer) {
+        return;
+    }
+    [data enumerateByteRangesUsingBlock:^(const void * _Nonnull bytes, NSRange byteRange, __unused BOOL * _Nonnull stop) {
+        memcpy(newbuffer + byteRange.location, bytes, byteRange.length);
+    }];
+    char *oldbuffer = *buffer;
+    *buffer = newbuffer;
+    free(oldbuffer);
+}
+
+//
+// Metadata is also stored on disk so that it is accessible at next launch if an OOM is detected.
+//
+- (void)writeData:(NSData *)data toFile:(NSString *)file {
+    self.pendingWrite = data;
+
+    dispatch_async(BSGGlobalsFileSystemQueue(), ^{
+        NSData *pendingWrite;
+
+        @synchronized (self) {
+            if (!self.pendingWrite) {
+                // The latest data has already been written to disk.
+                return;
+            }
+            pendingWrite = self.pendingWrite;
+        }
+
+        NSError *error = nil;
+        if (![pendingWrite writeToFile:(NSString *_Nonnull)file options:NSDataWritingAtomic error:&error]) {
+            bsg_log_err(@"%s: %@", __FUNCTION__, error);
+        }
+
+        @synchronized (self) {
+            if (self.pendingWrite == pendingWrite) {
+                self.pendingWrite = nil;
+            }
+        }
+    });
+}
+
 @end

Bugsnag/include/Bugsnag/BugsnagMetadata.h

@@ -28,7 +28,19 @@
 
 #import <Bugsnag/BugsnagMetadataStore.h>
 
+NS_ASSUME_NONNULL_BEGIN
+
 /// :nodoc:
 @interface BugsnagMetadata : NSObject <BugsnagMetadataStore>
-- (instancetype _Nonnull)initWithDictionary:(NSDictionary *_Nonnull)dict;
+
+- (instancetype)initWithDictionary:(NSDictionary *)dict;
+
+/// Configures the metadata object to serialize itself to the provided buffer and file immediately, and upon each change.
+- (void)setStorageBuffer:(char *_Nullable *_Nullable)buffer file:(nullable NSString *)file;
+
+/// Exposed to facilitate unit testing.
+- (void)writeData:(NSData *)data toBuffer:(char *_Nullable *_Nonnull)buffer;
+
 @end
+
+NS_ASSUME_NONNULL_END

CHANGELOG.md

@@ -1,13 +1,20 @@
 Changelog
 =========
 
-## 6.9.7 (2021-06-23)
+## TBD
 
 ### Bug fixes
 
+* Improve performance of adding metadata by using async file I/O.
+  [#1126](https://github.com/bugsnag/bugsnag-cocoa/pull/1126)
+
 * Improve performance of leaving breadcrumbs by using async file I/O.
   [#1124](https://github.com/bugsnag/bugsnag-cocoa/pull/1124)
 
+## 6.9.7 (2021-06-23)
+
+### Bug fixes
+
 * Prevent some potential false positive detection of app hangs.
   [#1122](https://github.com/bugsnag/bugsnag-cocoa/pull/1122)