oblador / react-native-vector-icons

Customizable Icons for React Native with support for image source and full styling.
https://oblador.github.io/react-native-vector-icons/
MIT License
17.31k stars 2.12k forks source link

Fix for Memory Leak. iOS. RNVectorIconsManager #1568

Closed kuserhii closed 7 months ago

kuserhii commented 8 months ago

Released errorRef object which is not managed by ARC to fix memory leak

Environment

Xcode 14.3, iOS 16.4

Opened issue

https://github.com/oblador/react-native-vector-icons/issues/1499

Description

Into loadFontWithFileName func we call CTFontManagerRegisterGraphicsFont whitch have parameter with type CFErrorRef * which is not managed by ARC.

Screenshot 2023-05-18 at 6 47 16 PM Screenshot 2023-05-18 at 6 47 45 PM

Proposed fix

Proposed fix in RNVectorIconsManager.m:

To avoid memory leak we need to release that errorRef varible after usage.

RCT_EXPORT_METHOD(loadFontWithFileName:(NSString *)fontFileName
                  extension:(NSString *)extension
                  resolver:(RCTPromiseResolveBlock)resolve
                  rejecter:(RCTPromiseRejectBlock)reject)
{
  NSBundle *bundle = [NSBundle bundleForClass:[self class]];
  NSURL *fontURL = [bundle URLForResource:fontFileName withExtension:extension];
  NSData *fontData = [NSData dataWithContentsOfURL:fontURL];

  CGDataProviderRef provider = CGDataProviderCreateWithCFData((CFDataRef)fontData);
  CGFontRef font = CGFontCreateWithDataProvider(provider);

  if (font) {
    CFErrorRef errorRef = NULL;
    if (CTFontManagerRegisterGraphicsFont(font, &errorRef) == NO) {
      NSError *error = (__bridge NSError *)errorRef;
      if (error.code == kCTFontManagerErrorAlreadyRegistered) {
        resolve(nil);
      } else {
        reject(@"font_load_failed", @"Font failed to load", error);
      }
    } else {
      resolve(nil);
    }
    if (errorRef) {
        CFRelease(errorRef);
    }
    CFRelease(font);
  }
  if (provider) {
    CFRelease(provider);
  }
} 

Here we add

if (errorRef) {         
   CFRelease(errorRef);     
} 

Reproducible Demo

Test project with memory leak https://github.com/kuserhii/ReactNativeVectorIconMemoryLeak

johnf commented 7 months ago

@oblador This looks reasonable but I don't know enough about swift and couldn't find any docs on who is responsible for the memory release. Can you take a look?