Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Memory Leak. iOS. RNVectorIconsManager #1568

Merged

Conversation

kuserhii
Copy link
Contributor

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

Environment

Xcode 14.3, iOS 16.4

Opened issue

#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

Released errorRef object which is not managed by ARC to fix memory leak
@johnf
Copy link
Collaborator

johnf commented Nov 7, 2023

@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?

Copy link
Owner

@oblador oblador left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@oblador oblador merged commit 69841af into oblador:master Nov 9, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants