[@nativescript/imagepicker] Memory Allocation Bug

[sspark3]

Environment { "@angular/animations": "~10.1.4", "@angular/common": "~10.1.4", "@angular/compiler": "~10.1.4", "@angular/core": "~10.1.4", "@angular/forms": "~10.1.4", "@angular/platform-browser": "~10.1.4", "@angular/platform-browser-dynamic": "~10.1.4", "@angular/router": "~10.1.4", "@nativescript/angular": "10.1.4", "@nativescript/background-http": "^5.0.1", "@nativescript/camera": "^5.0.2", "@nativescript/core": "~7.1.0", "@nativescript/imagepicker": "^1.0.2", "@nativescript/theme": "~2.5.0", "@nativescript/types": "^7.0.4", "@nstudio/nativescript-checkbox": "^2.0.4", "nativescript": "^7.1.2", "nativescript-background-fetch": "^1.2.1", "nativescript-bitmap-factory": "^1.8.1", "nativescript-download-progress": "^1.2.0", "nativescript-mediafilepicker": "^4.0.1", "nativescript-pdf-view": "^2.0.1", "nativescript-ui-listview": "^9.0.4", "nativescript-webview-interface": "^1.4.3", "reflect-metadata": "~0.1.10", "rxjs": "~6.6.0", "tns-ios": "^6.5.2", "tslib": "1.10.0", "zone.js": "~0.11.1" }, "devDependencies": { "@angular-devkit/core": "~10.1.4", "@angular/cli": "~10.1.4", "@angular/compiler-cli": "~10.1.4", "@nativescript/ios": "7.1.1", "@nativescript/webpack": "~4.0.0", "@ngtools/webpack": "~10.1.4", "typescript": "~3.9.0" }

Describe the bug I created an app where users create checklists and upload images for different items. I am using imagepicker to accomplish this. With the latest iOS version, we noticed users experiencing crashing when uploading multiple images. Taking a deeper look, we saw memory was capping out causing the crashing. When the saveToFile function is called, it allocates memory to save, and then never releases it. When that line is commented out, the behavior is expected; memory goes up when the image is uploaded, and then goes right back down.

Code Associated

selectImages() {

let context = imagepicker.create({
  mode: "multiple",
  maximumNumberOfSelection: 10
});

context.authorize().then(() => {
  return context.present();
}).then((selection) => {

  const documents: Folder = <Folder>knownFolders.documents();
  const folder: Folder = <Folder>documents.getFolder("photos");
  let running = false;
  selection.forEach((selected) => {

    var newImage:ImageSource;
    ImageSource.fromAsset(selected).then((res) => {
      newImage = res;
      let newPath = folder.path + "/cameraPic_" + (new Date()).getTime();
      let newPathThumbnail = newPath + "_thumbnail.jpeg"
      newPath = newPath + ".jpeg";
      let photoJson = {path: "file://" + newPath,thumb: "file://" + newPathThumbnail}

      newImage.saveToFile(newPath,"jpeg");

      running = false;
    });        
  });
});

}

[triniwiz]

A fix for this was merged in core things should be fine once a version is out.

[crowers]

Is this now in the @next build? When is the next release due?

[sspark3]

@NathanWalker Is the fix you implemented in any way different than calling that releaseNativeObject ourselves outside of the nativeScript code base every-time a user up-loads an image? Referencing the code base above as to where we attempted calling this. We tried this and it did not seem to work. Also, when do you all expect this version to release? We currently have a client who has been down for over 4 weeks due to this bug, so time is of the essence.

[crowers]

@NathanWalker @sspark3 From my understanding, looking at the @triniwiz commit NativeScript/NativeScript@6f821b0 the fix cannot be applied outside of the saveToFile function as the native object we need to release is allocated by getImageData and the result is encapsulated within the function and not returned from it. So we have no variable available to us to pass releaseNativeObject from our code. Our customer's app under development for over a year cannot be released due to this issue. It would also be good to understand why iOS 14 is behaving differently with memory allocations, if indeed it is related to the release, to find out if there are other likely impacts. But our priority is being able to have a release of this fix at this point. Thanks all.

[sspark3]

Thanks for the input @crowers .... Glad to know we are not the only ones experiencing this issue over here. If there is any updates on this bug @NathanWalker please let us know. Trying to get a time frame out to our clients as soon as possible. Thank you guys!

[NathanaelA]

You should just try using @next and see if it fixes your issue. @next is a daily release, so anything merged should be in it.

[crowers]

Thanks Nathanael

We have tried that and checked again yesterday but @next for @nativescript/core doesn't contain this merged fix. We are really puzzled. We are trying to fork @next and add the fix manually as the customer is getting desperate.

[NathanWalker]

@crowers we had to back out Osei's fix in core as it caused a side effect but in the process we did find a workaround if you want to try it. Ultimately the code in question that leads to the cause is this:

const data = getImageData(this.ios, format, quality);
if (data) {
    return NSFileManager.defaultManager.createFileAtPathContentsAttributes(path, data, null);
}

the issue is that data is a native instance that doesn't get released in repetitive back-to-back operations quickly enough and thus causes the memory to climb. Osei's fix:

const data = getImageData(this.ios, format, quality);
if (data) {
  const result = NSFileManager.defaultManager.createFileAtPathContentsAttributes(path, data, null);
  // release native memory earlier
  releaseNativeObject(data);
  return result;
}

Fixed it in isolated cases but also most likely would cause you a different crash at times which would be a BAD_ACCESS type of crash.

We found while experimenting there that adding a timeout around the release did help:

const data = getImageData(this.ios, format, quality);
if (data) {
  const result = NSFileManager.defaultManager.createFileAtPathContentsAttributes(path, data, null);
  setTimeout(() => {
    // release native memory earlier
    releaseNativeObject(data);
  }, 20);
  return result;
}

However that can be frail as well. You can modify core modules by doing code node_modules/@nativescript/core to open in Vs Code and making that change in that spot to try it locally while developing. The other suggestion you could try is modifying that as follows:

// set the following before your app boots:
global.ImageDataCnt = 0;
global.ImageDataInstances = {};

// then modify that area of code as follows:
global.ImageDataCnt++;
global.ImageData[globa.ImageDataCnt] = getImageData(this.ios, format, quality);
if (global.ImageData[globa.ImageDataCnt]) {
  return NSFileManager.defaultManager.createFileAtPathContentsAttributes(path, global.ImageData[global.ImageDataCnt], null);
}

This would allow you to monitor the instance count in your routine which is calling that repetitively and begin manually releasing instances from memory, for example:

if (global.ImageDataCnt > 5) {
  for (const key in global.ImageDataInstances) { releaseNativeObject(global.ImageDataInstances[key]) }
}

This is more a temporary stop gap that you can try to help you get you out of a bind. We're looking into other ways to modify that particular area to help mitigate but try some of those suggestions and let us know when you can.

[crowers]

Thank you Nathan, that's helpful to know it's still being worked on.

We have tried several iterations of the fix you suggested today, but we are either seeing no improvement on the memory usage or it crashes if we call the release function.

We will update our customer, and look forward to further updates on a resolution.

[sspark3]

@crowers In case you are still having the issue, we managed to get a temporary fix for this by downgrading to the 6.5.4 iOS runtime. Garbage collection seems to correctly pick up the image data in this build. It is in the hands of our testers right now, but the initial results seem to be good.

[crowers]

Thanks @sspark3 - are you suggesting downgrade just the ios package to @nativescript/ios "JSC" / "6.5.4"? We tried that but it didn't work - app crashed on load. We've spent 2 months upgrading the project to NS7 battling through many problems along the way in order to get iOS 14 support, only to really disappoint the customer just before launch because of this memory leak. They are beginning to lose confidence in NS, and so are we. Rant over. We really can't unwind everything and go back to NS6 at this late stage, it would be a major fail. Guys, this memory bug is critical and needs fixing quickly somehow or other. If it can't be we need to understand more to explain to the customers what has happened here. It doesn't appear to be an issue with iOS 14, but the NS implementation of the v8 runtime, is that correct?

[rigor789]

@crowers what sort of crash are you getting with @nativescript/ios@6.5.4? And yes, it's a bug in ns-v8ios-runtime most likely, but finding it isn't trivial - otherwise it would've been already patched & released.

And I don't have a way of knowing your apps specifics, but a small memory leak - while pretty bad - could in many cases be tolerable to release.

[crowers]

Many thanks @rigor789 for confirmation - I will retest and confirm the crash details but we are losing 10-100Mb of memory depending on the image size - approx 50Mb each time we call saveToFile after ImageSource.fromAsset for a 1024x1024 pixel image. We are also reading the image again to collect EXIF metadata so that could be another area where memory is lost. I will put together a detailed summary of the use case for you to review if that would help. The application gathers photo evidence so allows multiple images to be picked or taken (1-100) in a short period of time before uploading them in the b/g. Are you saying we should be able to use CLI v7@latest, @nativescript/core@latest and @nativescript/ios@6.5.4 / @JSC ? I couldn't get a build without errors with this combo - or do we have to revert to the v6 codebase? Thanks in advance for your help.

[rigor789]

Are you saying we should be able to use CLI v7@latest, @nativescript/core@latest and @nativescript/ios@6.5.4 / @jsc ? I couldn't get a build without errors with this combo - or do we have to revert to the v6 codebase? Thanks in advance for your help.

Yes, CLI v7@latest, @nativescript/core@latest and @nativescript/ios@6.5.4 / @jsc work fine together, just make sure to ns clean and do a fresh build after you change to @nativescript/ios@6.5.4.

[crowers]

That's good news @rigor789. Using CLI v7@latest, @nativescript/core@latest and @nativescript/ios@6.5.4 Deployment target 12.1 The build successfully generates the iOS project using ns run ios with an iPhone X device specified. However it doesn't install the app onto the device and hangs. I can't build the app in Xcode either as it fails with "Use of undeclared identified TNSRuntime"

Any ideas?

[rigor789]

@crowers that's very strange, I've never seen anything like that. Would it be possible to share the project with me to take a look?

Another thing to check would be to create a fresh project, change runtime version to 6.5.4, ns clean & ns run ios and see if that one works fine (to eliminate possibility of xcode/system setup related issues). Next, I would check if there are any customizations in your project to the build.xcconfig in App_Resources/iOS that may affect the build?

[sspark3]

We experienced the same @crowers . You cannot build in Xcode but you can run it via the command line.

[crowers]

To fix "Use of undeclared identifier TNSRuntime" etc. We followed these instructions from @rigor789

1. ns clean
2. ns prepare ios
3. opened platforms/ios/ontrack.xcworkspace
4. ran from XCode to build
5. it failed to build
6. In XCode selected Product > Clean Build Folder and rebuilt and it succeeded

This was using the 6.5.4 (JSC) iOS runtime with @latest (v7) core and CLI.

Will test some more to see if the memory leak is fixed in 6.5.4.

[triniwiz]

This was fixed in the latest iOS runtime