olexale / arkit_flutter_plugin

ARKit Flutter Plugin
MIT License
797 stars 225 forks source link

Memory deallocation on controller dispose #89

Closed miles-au closed 3 years ago

miles-au commented 4 years ago

In the example app, if I continue to open and close multiple AR screens, the amount of memory used goes up every time I open a screen. It looks like the controller's dispose function pauses the session, but if I open a new page, it appears to initialize a new controller without deallocating the last one.

I need a similar functionality to the example app where the user can go in and out of the AR screens. Is there a workaround to this?

olexale commented 4 years ago

Hi @miles-au ,

Yeah, it seems that we have a leak in the plugin. I'll get back to this later.

Regards, Olexandr

miles-au commented 4 years ago

@olexale Is there anyway I could help? If you point me in the right direction, I could maybe at least make an attempt.

Initially I was thinking that this could be solved by setting the ARSCNView to nil on dispose, but it doesn't seem like a very nice solution. It would also require turning the sceneview variable into an optional.

James-A-White commented 4 years ago

I've also encountered this problem. Sometimes my app crashes, sometimes it corrupts the OS of the device to the extent that we have to reboot to get it to work again. I'm guessing that my problem is also related to a memory leak.

olexale commented 4 years ago

@miles-au , a few weeks earlier I profiled the sample app, and for me it looked like there is a problem in the Flutter native view bridging mechanism (it's still in preview). On the other hand, there are lots of other plugins that use the same mechanism, hence most probably the problem is somewhere in the plugin's code. My next step was to go with the profiler one more time, maybe I just missed something obvious, and after that to take, for example, google_maps_flutter plugin and compare the initialization and destruction code.

AlexShmukler commented 3 years ago

Hi @olexale! Facing a similar issue. If I open and close the same screen with ARKitSceneView for ~10 times the app crashes. Do you have any updates/workarounds on this? My flutter version is 1.17.5. Can changing the flutter version help?

BrutalCoding commented 3 years ago

I've got the same issue here, seems like the dispose method does not dispose properly. Any updates on this issue?

Doctor summary (to see all details, run flutter doctor -v): [✓] Flutter (Channel stable, 1.22.2, on Mac OS X 10.15.7 19H2, locale en-AU)

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.2) [✓] Xcode - develop for iOS and macOS (Xcode 12.0.1) [✓] Android Studio (version 4.0) [✓] VS Code (version 1.50.1) [✓] Connected device (2 available)

• No issues found!

So it seems like ARKIt only provides a .pause() method, no such thing as dispose. Thus I tried to set the sceneView to nil, which did not even compile for me for some reason.

In the initialize of FlutterArkitView, I tried the following:

if (configuration != nil) {
    if (self.sceneView.session.configuration != nil){
        self.sceneView.session.run(self.sceneView.session.configuration!)
    } else {
        self.sceneView.session.run(self.configuration!)
    }
} 

With the hope it would resume the sceneView instead of initialising a new one on top, but it seems like either my condition or the run itself are not making a difference.

I'll get back to this if I found the solution but it is rather tricky, I might also try to not dispose or pause the ARKit page (in Flutter) at all, just keeping it running in the background could be a workaround but I haven't tested this yet.

BrutalCoding commented 3 years ago

I've made a PR that fixed the issue, seems to work fine in my use cases which is opening/closing AR page several times, going to background and back and spawning a .dae node every time AR is opened.

See https://github.com/olexale/arkit_flutter_plugin/pull/112

olexale commented 3 years ago

This should be fixed in 0.6.0. Could you please check?

BrutalCoding commented 3 years ago

I've checked it and closed down my PR, your fix looks cleaner and more robust. This issue can be closed since the memory leak is no longer an issue. @olexale @miles-au