[TIMOB-25693] Android: Possible memory leak when setting ListSections on a ListView

[ypbnv]

JIRA: https://jira.appcelerator.org/browse/TIMOB-25693

Optional Description:
Release current section items when repopulating a list through setSection().

Test case:
Alloy application in the JIRA ticket.
Clicking multiple times on Open List and/or Repopulate list should not crash the application.

[ypbnv]

@jquick-axway Bump.

[garymathews]

I don't think this is necessary, releaseViews should only remove the reference to the listView. If release needs calling it should be done separately

[garymathews]

Maybe this should be releaseViews() ?

If we release the whole proxy then we can't re-add it again later

[lokeshchdhry]

@ypbnv ,Can you please look at the suggested changes by @garymathews .

[build]

	Messages
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

[ypbnv]

@garymathews @lokeshchdhry The proposed changes are correct, but not enough to eliminate the memory leak. I have been working on it, but so far I haven't been able to get it done. The sample application keeps references to ListSectionProxy instances that are created in the context only of repopulating a list through setSections() method and are not released afterwards (when replaced by a new set of sections). This does not seem to happen with other types of proxies so I am going through the templates code to try and find why is that happening.

I will update you once I have something working or at least in the right direction. I am not sure if we should keep it with a 7.1.0 milestone, though. It is hard for me to give an ETA.

[ypbnv]

@garymathews Thank you for solving this!

[jquick-axway]

There's a race condition here where the weak reference can be collected between the 2 get() calls. It's slim, but it can happen. The following would solve it.

KrollProxySupport proxy = (proxySupport != null) ? proxySupport.get() : null;
if (proxy != null) {
	proxy.onEventFired(event, data);
}

Not trying to be picky here, but I've seen Java code get burned by issues like this in the past. We should do the same in the setHasListenersForEventType() method.

[jquick-axway]

Question:
Calling releaseViews() will release the section. Will this put the JavaScript ListSection object in a bad state and prevent the developer from re-adding the section to the ListView?

[garymathews]

Here's another test case:

const NUM_SECTIONS = 50, NUM_ITEMS_PER_SECTION = 100;

var win = Ti.UI.createWindow({backgroundColor: 'white'}),
    listView = Ti.UI.createListView({
        templates: {
            template: {
                childTemplates: [
                    {
                        type: 'Ti.UI.Label',
                        bindId: 'label',
                        properties: {
                            color: 'black'
                        }
                    }
                ]
            }
        },
        defaultItemTemplate: 'template'
    }),
    fillListView = Ti.UI.createButton({ title: 'FILL', bottom: 5, left: 5 }),
    clearListView = Ti.UI.createButton({ title: 'CLEAR', bottom: 5, right: 5 }),
    sections = [];

function populateSections () {
    sections = [];

    for (let i = 0; i <= NUM_SECTIONS; i++) {
        let section = Ti.UI.createListSection({ headerTitle: `SECTION #${i + 1}` }),
            items = [];

        for (let j = 0; j <= NUM_ITEMS_PER_SECTION; j++) {
            items.push({ label: { text: `ITEM ${j + 1}` } });
        }
        section.setItems(items);

        sections.push(section);
    }
}

fillListView.addEventListener('click', function () {
    populateSections();
    listView.setSections(sections);

    // lets test re-adding sections
    listView.setSections([]);
    listView.setSections(sections);
});

clearListView.addEventListener('click', function () {
    sections = [];
    listView.setSections(sections);
});

win.add([ listView, fillListView, clearListView ]);
win.open();

[lokeshchdhry]

FR Passed.

Current sections are released when repopulating & there does not seen to be a memory leak & the app does not crash due to out of memory issues.

Studio Ver: 5.0.0.201712081732
SDK Ver: 7.2.0 local build
OS Ver: 10.13.2
Xcode Ver: Xcode 9.2
Appc NPM: 4.2.12
Appc CLI: 7.0.2
Daemon Ver: 1.0.1
Ti CLI Ver: 5.0.14
Alloy Ver: 1.11.0
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 6P --- Android 8.0.0
⇨ google Nexus 5 --- Android 6.0.1

[peterli888]

it seem like not resolve ,i test in android 7.1.2
i use :"adb shell dumpsys meminfo com.xxx.yyy" to check mem leak,
you will see this in Object section:
Views:0, Activities:0 ,means no memory leak,this is a window without listview,

if a window has a listview with 2 section, open it ,close it(exit the app),
run :adb shell dumpsys meminfo com.xxx.yyy
i get : Views:40, Activities:1,
open,close other time,get: Views:80, Activities:2,
open,close other time,get: Views:120, Activities:3,
open,close other time,get: Views:160, Activities:4,
it will grow not stop,it is leak.TOTAL memory will grow.

and, Views num related to sections.length,
sections.length=0,views=0, no mem leak,
sections.length=1,views=30,
sections.length=2,views=40,
sections.length=3,views=50,
it means one section left 10 view not release,
so,the leak may be in ui.widget.listview.ListSectionProxy.java,
but ,i checked all the things seems like need release and release them,but not work.

i test with: 6.3.0.GA,7.2.0,7.1.x, my own build 7.1.x, ypbnv/titanium_mobile , all leak,

my app.js,no listview.setSections(xyz)

var win = Ti.UI.createWindow({backgroundColor: 'white'});
var listView = Ti.UI.createListView();
var sections = [];

var fruitSection = Ti.UI.createListSection({ headerTitle: 'Fruits'});
var fruitDataSet = [
{properties: { title: 'Apple'}},
{properties: { title: 'Banana'}},
];
fruitSection.setItems(fruitDataSet);
sections.push(fruitSection);

var vegSection = Ti.UI.createListSection({ headerTitle: 'Vegetables'});
var vegDataSet = [
{properties: { title: 'Carrots'}},
{properties: { title: 'Potatoes'}},
];
vegSection.setItems(vegDataSet);
sections.push(vegSection);

listView.sections = sections;
win.add(listView);
win.open();

[mas222]

we are still having this issue, the original fix posted before the requested changes seemed to work when we tested them locally, but 7.1.0 GA still leaks