Titanium JIRA Archive
Titanium SDK/CLI (TIMOB)

[TIMOB-27207] TableView / TableViewRow memory leak

GitHub Issuen/a
TypeBug
PriorityCritical
StatusClosed
ResolutionFixed
Resolution Date2019-10-15T11:11:11.000+0000
Affected Version/sRelease 8.0.0, Release 8.0.1
Fix Version/sRelease 8.2.1
ComponentsiOS
Labelsn/a
ReporterVDLP
AssigneeJan Vennemann
Created2019-06-28T14:06:11.000+0000
Updated2020-06-02T01:29:26.000+0000

Description

Old table row references are not cleared when calling *tableView.setData(rows)*. In a production app running SDK 7.4.1.GA this is causing serious memory issues. Because the row structure is quite complex, it's important that old rows are completely cleaned from memory before new ones are set. We've been able to reproduce this issue in a example application for multiple SDK's (8.0.0.GA, 7.4.1.GA, 8.0.1.GA). As demonstrated in the attached video you can see that the objects are not cleared from memory. The source code for the app running in the video has also been attached to this issue. We've tried calling $.off();$.destroy(); on the controllers/imagerow controller, this does not destroy the proxies.

Attachments

FileDateSize
app.zip2019-06-28T13:57:18.000+00008445019
Findings for TIMOB-27207.pdf2019-10-10T12:05:46.000+000068678
invalid-references.mov2019-06-28T13:56:49.000+00001371563
memleak8.0.2.GA.mp42019-07-03T08:41:25.000+0000354404
Memory Leak Monitoring.mov2019-07-04T05:23:41.000+00006690669

Comments

  1. Rakhi Mitro 2019-06-29

    [~menzo], Thanks for sharing with us. Are you experiencing this for both iOS and android platform?
  2. VDLP 2019-07-01

    Currently testing this on iOS. Not sure how to accurately profile Android. For our production app, the memory issues also occur on Android.
  3. VDLP 2019-07-03

    Any updates on this? :)
  4. Rakhi Mitro 2019-07-03

    [~menzo], Thanks for your feedback. Can you please test this on 8.0.2.GA and let us know the results?
  5. VDLP 2019-07-03

    We've already tested this on 8.0.2.GA. Same issues, the Proxy's are not removed.
  6. VDLP 2019-07-03

    [^memleak8.0.2.GA.mp4]
  7. Rakhi Mitro 2019-07-03

    [~menzo], Are you experiencing the same behaviour also in device also? Can you please check that?
  8. VDLP 2019-07-03

    Yes.
  9. Jan Vennemann 2019-07-05

    PR (master): https://github.com/appcelerator/titanium_mobile/pull/11028 PR (8_1_X): https://github.com/appcelerator/titanium_mobile/pull/11029 [~menzo], you may want to try this line before you update table.data as a workaround: 
       table.data.forEach(s => s.rows.forEach(r => r));
   table.data = yourTableData;
  10. Rakhi Mitro 2019-07-07

    [~menzo], Hope you are doing fine today. Would you please reply us regarding previous reviews? We are looking forward to your response.
  11. VDLP 2019-07-08

    Fix suggested by Jan doesn't work on *7.4.1.GA* & *8.0.2.GA* 
        table.data.forEach(s => s.rows.forEach(r => r));
    table.data = yourTableData;
    Our app is running on 7.4 so upgrading to 8.1.X isn't really a option at this moment. Some more info on how the above code snippet is supposed fix our problem would be great.
  12. Jan Vennemann 2019-07-09

    [~menzo], see the ticket for a in-depth explanation. Long story short, accessing the sections and rows in JavaScript once will make sure they get properly garbage collected like they should. Here is a longer example which should make it more clear how to apply this workaround: 
        const win = Ti.UI.createWindow({
      layout: 'vertical'
    });
    const table = Ti.UI.createTableView({
      data: [ {title: 'Iron Man'}, {title: 'Deadpool'}, {title: 'Star-Lord'}, {title: 'Venom'} ]
    });
    // Workaround (1): access table data after initially creating the table ...
    table.data.forEach(s => s.rows.forEach(r => r));
    const btn = Ti.UI.createButton({
      title: 'Reload',
      top: 40
    });
    btn.addEventListener('click', () => {
      table.data = [ {title: 'Iron Man'}, {title: 'Deadpool'}, {title: 'Star-Lord'}, {title: 'Venom'} ];
      // Workaround (2): ... or whenever setting new table data.
      table.data.forEach(s => s.rows.forEach(r => r));
    });
    win.add(btn);
    win.add(table);
    win.open();
    Note that it may take a while for the JS garbage collection to kick in so you won't notice an immediate change in Instruments.
  13. Jan Vennemann 2019-10-10

    PR (8_2_X): https://github.com/appcelerator/titanium_mobile/pull/11274
  14. Sohail Saddique 2019-10-10

    Thought I'd give this a go, but for me FR: Failed. Please see attached PDF of findings. [^Findings for TIMOB-27207.pdf]
  15. Lokesh Choudhary 2019-10-14

    PR's merged.
  16. Sohail Saddique 2019-10-15

    Verified on builds 8.2.1.v20191014114554 and 8.3.0.v20191014114038. Ticked closed.

JSON Source