apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.15k stars 987 forks source link

(ios) fix: memory leak when removing the CDVViewController #1326

Closed cbernier closed 1 year ago

cbernier commented 1 year ago

Platforms affected

iOS

Motivation and Context

I am using Cordova inside a React Native application so I often need to destroy and re-create the CDVViewController.

Description

Since I am using a custom scheme, it is leaking (the CDVViewController is never deallocated despite being remove from its parentViewController)

Testing

I confirm that by changing strong to weak, the CDVViewController is deallocated as soon as it is removed from its parent view controller.

Checklist

dpogue commented 1 year ago

This sounds like it solves https://github.com/apache/cordova-ios/issues/1071, right?

And then https://github.com/apache/cordova-ios/issues/920 is a different, unrelated memory leak?

cbernier commented 1 year ago

Oh right, it should fix those 2 issues, at least for me there's no more leak: both the webView and view controller get deallocated.

codecov-commenter commented 1 year ago

Codecov Report

Merging #1326 (7c5ff48) into master (c000a1b) will decrease coverage by 0.04%. The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1326      +/-   ##
==========================================
- Coverage   78.64%   78.61%   -0.04%     
==========================================
  Files          15       15              
  Lines        1789     1791       +2     
==========================================
+ Hits         1407     1408       +1     
- Misses        382      383       +1     
Impacted Files Coverage Δ
lib/create.js 94.59% <66.66%> (-1.24%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more