[TIMOB-19891] Android: Memory Leak on Window

GitHub Issue	n/a
Type	Bug
Priority	Critical
Status	Closed
Resolution	Fixed
Resolution Date	2016-03-07T19:04:13.000+0000
Affected Version/s	n/a
Fix Version/s	Release 5.2.1
Components	Android
Labels	android
Reporter	Andrey Tkachenko
Assignee	Ashraf Abu
Created	2015-08-04T20:53:39.000+0000
Updated	2016-03-08T20:19:03.000+0000

Description

I open some windows and then close all one by one except first one. In 4.1.0.GA more then one WindowProxy object remain in memory but must only one. In 3.5.1.GA remain two objects. In the iphone only first one.

(function openWindow(n) {

    var win = Ti.UI.createWindow({
        backgroundColor : "#aaaaaa",
        layout : "vertical"        
    });    

    win.title = "Window # " + n;

    var label = Ti.UI.createLabel({
        text : "Window # " + n,
        color : "#ffffff",
        top : 20
    });
    win.add(label);

    var openButton = Ti.UI.createButton({
        title : "Open",
        color : "#ffffff",
        top : 20
    });
    openButton.addEventListener('click', function onOpenByButton(evt) {
        openWindow(n + 1);
    });

    win.add(openButton);

    var closeButton = Ti.UI.createButton({
        title : "Close",
        color : "#ffffff",
        top : 20
    });
    closeButton.addEventListener('click', function onCloseByButton(evt) {
        win.close();
        win = null;
    });

    win.add(closeButton);

    win.open();
   
})(1);

May be related to this changes https://github.com/appcelerator/titanium_mobile/commit/2b12ad61775920f8045e168ae678ce123dfa14b4#diff-034a9360d01584987d9c951c0a215f65 !memoryleak.png|thumbnail!

Attachments

File	Date	Size
3.5.1.GATest.png	2015-08-11T09:21:20.000+0000	147972
4.1.0GATest.png	2015-08-11T09:21:23.000+0000	163785
5.2.1.v20160303112058.png	2016-03-04T00:23:05.000+0000	148543
memoryleak.png	2015-08-04T20:53:09.000+0000	81555
With SDK 4.2.0.v20150807112024.png	2015-08-12T22:34:04.000+0000	197172

Comments

Lokesh Choudhary 2015-08-12

The memory leak is seen on window with the above code in the description. Attached screenshot for reference. Environment: Appc Studio : 4.2.0.201508062204 Ti SDK : 4.2.0.v20150812103137 Ti CLI : 4.1.4 Alloy : 1.6.2 MAC Yosemite : 10.10.4 Appc NPM : 4.1.0 Appc CLI : 4.2.0-44 Node: v0.10.37 Android Emulator : android 4.4.2 Node : v0.10.37
Radamantis Torres-Lechuga 2015-08-20

From my perspective, this leak it's because the construction of the test case, we have eventlisteners inside the window constructor, and the functions on the eventlistener are not outside the window constructor, to fully check this memory leak, we need a test case following best practices
Andrey Tkachenko 2015-09-04

Is anybody here?
Andrey Tkachenko 2015-09-14

Another use case for you if my first example seems bad for you: Open and then close second window (simple empty window) from index controller in Alloy. When use 3.5.1.GA no one WindowProxy is present in memory after window closed. When use 4.x.x WindowProxy and it native objects remain in memory. Guys anybody can say something?

Andrey Tkachenko 2015-10-01

Ok look at another simple example and run it in 3.5.1.GA then 4.x.x etc. 1. create dump on root window 2. open first test window 3. open second test window 4. close second 5. close first 6. GC 7. create second dump and diff with first dump

      
   var win = Ti.UI.createWindow({
   	backgroundColor : "#aaaaaa",
   	title : "root window",
   	layout: "vertical"
   });
   
   var openButton = Ti.UI.createButton({
   	title : "Open 1",
   	color : "#ffffff",
   	top : 20
   });
   openButton.addEventListener('click', function(evt) {
   	var win = Ti.UI.createWindow({
   		backgroundColor : "#ff0000",
   		title : "test win 1"
   	});
   	var openButton = Ti.UI.createButton({
   		title : "Open 2",
   		color : "#ffffff",
   		top : 20
   	});
   	openButton.addEventListener('click', function(evt) {
   		Ti.UI.createWindow({
   			backgroundColor : "#00ff00",
   			title : "test win 2"
   		}).open();
   	});
   	win.add(openButton);
   	win.open();
   	win = null;
   	openButton = null;
   });
   
   win.add(openButton);
   
   openButton = null;
   
   win.open();

Fokke Zandbergen 2015-11-11

So you're saying you always need to remove all event listeners? I haven't seen that to be required. We might want to provide some more guiding in the docs on this and back that up with tests.
Andrey Tkachenko 2015-11-11

I saying that it not caused leaks before SDK 4.0.0. And issue not in listeners, please read carefuly my last comment.

Jebun Naher 2015-11-12

Hello We tested this issue in our environment and observed memory leak happens. Steps to test: - We run the app for the first time and created dump 1 - In the next step we run the app, opened window2 and window 3 - Used android back button to close window 2 - And created dump 2 - [Dump 1](http://postimg.org/image/hwuspceo9/) - [Dump 2](http://postimg.org/image/wt3rhkhb5/) *Testing Environment:* Appcelerator Command-Line Interface, version 5.0.4 Appcelerator Studio, build: 4.3.3.201510212245 Ti SDK: 5.0.2 GA Java Development Kit : 1.7.0_65 Node.js Version : 0.12.7 Mac Osx: 10.9.5 Android Emulator: Samsung Galaxy S4(4.4.4) Test Case:

   var main_win = Ti.UI.createWindow({
   	backgroundColor : "#aaaaaa",
   	title : "root window",
   	layout : "vertical"
   });
   var main_Button = Ti.UI.createButton({
   	title : "Open 1",
   	color : "#ffffff",
   	top : 20
   });
   main_Button.addEventListener('click', test);
   main_win.add(main_Button);
   main_win.open();
   var win2 = Ti.UI.createWindow({
   	backgroundColor : "#ff0000",
   	title : "test win 2"
   });
   var button2 = Ti.UI.createButton({
   	title : "Open 2",
   	color : "#ffffff",
   	top : 20
   });
   button2.addEventListener('click', f2);
   win2.add(button2);
   function test() {
   	win2.open();
   	win2.addEventListener('androidback', function() {
   		main_Button.removeEventListener('click', test);
   		win2.close();
   		win2 = null;
   	});
   };
   
   function f2() {
   	var win3 = Ti.UI.createWindow({
   		backgroundColor : "#00ff00",
   		title : "test win 3"
   	});
   	win3.open();
   
   	setTimeout(function() {
   		button2.removeEventListener('click', f2);
   		win3.close();
   		win3 = null;
   	}, 1000);
   	
   };

Thanks.

Andrey Tkachenko 2016-02-05

Whether there is any progress?
Andrey Tkachenko 2016-02-05

Please use instructions from my comment at: 01/Oct/15 10:22 AM And take dumps on single app session. We will be so for a long time to fix this bug :( Year or two?
Rene Pot 2016-02-17

Any update on this issue?
Andrey Tkachenko 2016-02-18

I investigated changes and found that you need delete *static* from here https://github.com/appcelerator/titanium_mobile/blob/5_2_X/android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java#L82 This changes will solve this bug, tested with my last test example. Will be awesome to apply fix to the Ti.SDK 5.2.0
Manuel Lehner 2016-02-18

[~falko] Can you send a pull request for this?
Andrey Tkachenko 2016-02-18

[PR](https://github.com/appcelerator/titanium_mobile/pull/7753)
Ashraf Abu 2016-02-19

[~falko] Thanks for the PR. Putting this in a sprint that's coming right up to get this reviewed.

Ashraf Abu 2016-02-24

Additional test to run from TIMOB-18956 :-

    var win = Titanium.UI.createWindow({  
        title:'Window',
        backgroundColor:'#fff'
    });
     
    win.orientationModes = [ 
        Titanium.UI.PORTRAIT, 
        Titanium.UI.UPSIDE_PORTRAIT,
        Titanium.UI.LANDSCAPE_LEFT,
    	Titanium.UI.LANDSCAPE_RIGHT
    ];
     
    Ti.Gesture.addEventListener('orientationchange', function(e){
        switch(e.orientation) {
        case Ti.UI.LANDSCAPE_RIGHT:
            Ti.API.info("LANDSCAPE_RIGHT");
    		Ti.UI.createNotification({
    	    	message:"LANDSCAPE_RIGHT",
    	    	duration: Ti.UI.NOTIFICATION_DURATION_SHORT
    		}).show();
            break;
        case Ti.UI.LANDSCAPE_LEFT:
            Ti.API.info("LANDSCAPE_LEFT");
    		Ti.UI.createNotification({
    	    	message:"LANDSCAPE_LEFT",
    	    	duration: Ti.UI.NOTIFICATION_DURATION_SHORT
    		}).show();
            break;
        case Ti.UI.UPSIDE_PORTRAIT:
            Ti.API.info("UPSIDE_PORTRAIT");
    		Ti.UI.createNotification({
    	    	message:"UPSIDE_PORTRAIT",
    	    	duration: Ti.UI.NOTIFICATION_DURATION_SHORT
    		}).show();
            break;
        case Ti.UI.PORTRAIT:
            Ti.API.info("PORTRAIT");
    		Ti.UI.createNotification({
    	    	message:"PORTRAIT",
    	    	duration: Ti.UI.NOTIFICATION_DURATION_SHORT
    		}).show();
            break;
        default:
            Ti.API.info("Unknown!!");
    	} 
    });
     
    win.open();

Ashraf Abu 2016-02-24

PR merged into 5_2_X (https://github.com/appcelerator/titanium_mobile/pull/7753) Cherry-picked commit to master: https://github.com/appcelerator/titanium_mobile/pull/7774
Ashraf Abu 2016-02-24

[~falko] Thank you.
Lokesh Choudhary 2016-03-03

Verified the memory leak with SDK 5.2.1.v20160303112058. After opening the window 10 times & closing all but the first I still see 7 windowProxy objects in the heap dump. Reopening. Environment: Appc Studio : 4.5.0.201602170831 Ti SDK : 5.2.1.v20160303112058 Ti CLI : 5.0.6 Alloy : 1.7.33 MAC El Capitan : 10.11.13 Appc NPM : 4.2.3 Appc CLI : 5.2.0 Node: 4.2.2 Nexus 6P - Android 6.0.1
Ashraf Abu 2016-03-04

[~lchoudhary] I tried it with master branch 5_2_X and not able to see 7 windowProxy objects leaking. Could you share the code you are using that would result in that? Is there any difference?
Lokesh Choudhary 2016-03-04
Ashraf Abu 2016-03-07

[~lchoudhary] I tried it with 5.2.1.v20160303112058 and found no issues. I think you might have missed just one final step in the test. After closing everything, the objects will still be there until you Cause GC. It won't be collected immediately. Please press Cause GC in the Android Device Monitor and then get the hprof. This should result in the items being collected and no objects leaking. If this is the cause, and you can verify that this works, please resolve this ticket. :) If not, please let me know. I've tried it by creating a hprof file before, after and after GC and found it to be working correctly with proxy objects 1, 10 and 1 respectively. Works correctly for me.
Ashraf Abu 2016-03-07

I'm using a Nexus 6 with Android 6.0.

Andrey Tkachenko 2016-03-07

I changed my test code from description. And tested it with 5.1.2 (patched with my PR from here). I opened windows up to "Window #10" and pushed to the GC button until memory stabilized. Updated code (from description):

    (function openWindow(n) {
    	var win = Ti.UI.createWindow({
    		backgroundColor : "#aaaaaa",
    		layout : "vertical"
    	});
    
    	win.title = "Window # " + n;
    
    	var label = Ti.UI.createLabel({
    		text : "Window # " + n,
    		color : "#ffffff",
    		top : 20
    	});
    
    	win.add(label);
    	label = null;
    
    	var openButton = Ti.UI.createButton({
    		title : "Open",
    		color : "#ffffff",
    		top : 20,
    		_n: n
    	});
    
    	openButton.addEventListener('click', function onOpenByButton(evt) {
    		openWindow(evt.source._n + 1);
    	});
    
    	win.add(openButton);
    	openButton = null;
    
    	var closeButton = Ti.UI.createButton({
    		title : "Close",
    		color : "#ffffff",
    		top : 20
    	});
    
    	closeButton.addEventListener('click', function onCloseByButton(evt) {
    		evt.source.getParent().close();
    	});
    
    	win.add(closeButton);
    	closeButton = null;
    	win.open();
    	win = null;
    })(1);

Screenshot: https://yadi.sk/i/AhGS0fDCpxgih *Please replace link to image* I think something wrong with closing process of Activities. Please spend time to find and fix it. Situation is compounded when we have more complex views hierarchy. I redid a lot of test for that in alloy and I found that many objects remains in memory for each Window that opend at least once. If you have many other windows this substantially.

Lokesh Choudhary 2016-03-07

[~msamah], Thanks, getting hprof after GC reduced the objects to 1or 2 instead of 7. Resolving the ticket & closing. Environment: Appc Studio : 4.5.0.201602170831 Ti SDK : 5.2.1.v20160303223838 Ti CLI : 5.0.6 Alloy : 1.7.33 MAC El Capitan : 10.11.13 Appc NPM : 4.2.3 Appc CLI : 5.2.0 Node: 4.2.2 Nexus 6P - Android 6.0.1
Ashraf Abu 2016-03-08

[~falko] I presume you are implying that there is something else that is leaking that is not in the original PR that you did? Is there a difference with alloy and classic as mentioned by you?
Andrey Tkachenko 2016-03-08

@Ashraf Abu Yes there is something else besides my PR. I am not developing classic app, and you can see in my previevs comment that some objects remained in memory. In Alloy we are use BaseController that holds references to the views. I think TiActivity is destroyed before controller instance and this may be the reason activity and its children views are not processed by GC. Something holds references to the native views. For exemple look at https://yadi.sk/i/r1gxTIs4pzJcT App: https://yadi.sk/d/WICFdTLPpzK3U Step to reproduce: Run in Android 1. GC, GC, GC, Create dump 1 2. Click to "Open win1" 3. Close with Back button 4. Click to "Open win2" 5. Close with Back button 6. Click to "Open win3" 7. Close with Back button 8. Click to "Open win2" 9. Close with Back button 10. Click to "Open win1" 11. Close with Back button 12. GC, GC, GC, Create dump 2

JSON Source