Free nanopb mallocs when losing references to them (#11139)

* Free nanopb mallocs when losing references to them

* style

* add changelogs

* Fix missing free

Author: morganchen12

Crashlytics/CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 - [fixed] Updated upload-symbols to 3.15. Disabled dSYM uploads for Flutter apps building with --obfuscate and added instructions for uploading through the Crashlytics dashboard. (#11136)
+- [fixed] Fixed a memory leak when generating session events (#11027).
 
 # 10.7.0
 - [fixed] Updated upload-symbols to 3.14 with an improvement to upload all dSYM files for Flutter apps

FirebasePerformance/CHANGELOG.md

@@ -1,3 +1,6 @@
+# Unreleased
+- [fixed] Fixed a memory leak when generating session events (#11027).
+
 # 10.6.0
 - [added] Integrated with Firebase sessions library and replaced the session information associated with performance metrics with session details provided by the Firebase sessions library. If your app uses the Performance Monitoring SDK, review Firebase's [data disclosure page](https://firebase.google.com/docs/ios/app-store-data-collection) to make sure that your app's privacy details in the App Store are accurate and complete.
 - [changed] Changed definition of sessions, as Performance Monitoring now depends on the new Firebase sessions SDK.

FirebaseSessions/CHANGELOG.md

@@ -0,0 +1,2 @@
+Firebase Sessions does not keep a changelog since it's only an
+independent module for code-sharing purposes.

FirebaseSessions/Sources/SessionStartEvent.swift

@@ -74,8 +74,28 @@ class SessionStartEvent: NSObject, GDTCOREventDataObject {
       .performance = firebase_appquality_sessions_DataCollectionState_COLLECTION_SDK_NOT_INSTALLED
   }
 
+  deinit {
+    let garbage: [UnsafeMutablePointer<pb_bytes_array_t>?] = [
+      proto.session_data.session_id,
+      proto.session_data.first_session_id,
+      proto.session_data.firebase_installation_id,
+      proto.application_info.app_id,
+      proto.application_info.session_sdk_version,
+      proto.application_info.device_model,
+      proto.application_info.development_platform_name,
+      proto.application_info.development_platform_version,
+      proto.application_info.apple_app_info.bundle_short_version,
+      proto.application_info.apple_app_info.mcc_mnc,
+    ]
+    for pointer in garbage {
+      nanopb_free(pointer)
+    }
+  }
+
   func setInstallationID(installationId: String) {
+    let oldID = proto.session_data.firebase_installation_id
     proto.session_data.firebase_installation_id = makeProtoString(installationId)
+    nanopb_free(oldID)
   }
 
   func setSamplingRate(samplingRate: Double) {
@@ -109,7 +129,9 @@ class SessionStartEvent: NSObject, GDTCOREventDataObject {
                                    appInfo: ApplicationInfoProtocol) {
     switch subscriber {
     case .Performance:
+      let oldString = proto.application_info.apple_app_info.mcc_mnc
       proto.application_info.apple_app_info.mcc_mnc = makeProtoString(appInfo.mccMNC)
+      nanopb_free(oldString)
       proto.application_info.apple_app_info.network_connection_info
         .network_type = convertNetworkType(networkType: appInfo.networkInfo.networkType)
       proto.application_info.apple_app_info.network_connection_info

FirebaseSessions/SourcesObjC/NanoPB/FIRSESNanoPBHelpers.h

@@ -32,6 +32,9 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+/// Deinitializes a nanopb struct. Rewritten here to expose to Swift, since `pb_free` is a macro.
+void nanopb_free(void* _Nullable);
+
 /// Returns an error associated with the istream. Written in Objective-C because Swift does not
 /// support C language macros
 NSString* FIRSESPBGetError(pb_istream_t istream);

FirebaseSessions/SourcesObjC/NanoPB/FIRSESNanoPBHelpers.m

@@ -28,6 +28,10 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+void nanopb_free(void *_Nullable ptr) {
+  pb_free(ptr);
+}
+
 NSError *FIRSESMakeEncodeError(NSString *description) {
   return [NSError errorWithDomain:@"FIRSESEncodeError"
                              code:-1
@@ -41,6 +45,10 @@
 // It seems impossible to specify the nullability of the `fields` parameter below,
 // yet the compiler complains that it's missing a nullability specifier. Google
 // yields no results at this time.
+//
+// Note 4/17/2023: The warning seems to be spurious (pb_field_t is a non-pointer
+// type) and is not present on Xcode 14+. This pragma can be removed after the
+// minimum supported Xcode version is above 14.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wnullability-completeness"
 NSData *_Nullable FIRSESEncodeProto(const pb_field_t fields[],
@@ -86,7 +94,7 @@
   if (pbBytes == NULL) {
     return NULL;
   }
-  memcpy(pbBytes->bytes, [data bytes], data.length);
+  [data getBytes:pbBytes->bytes length:data.length];
   pbBytes->size = (pb_size_t)data.length;
   return pbBytes;
 }