Opened 9 years ago

Closed 9 years ago

#11587 closed defect (fixed)

Subscription in Dialog._setup causes memory leak

Reported by: Roberto Mosqueda Owned by: bill
Priority: high Milestone: 1.5.1
Component: Dijit Version: 1.5
Keywords: memory leak Cc: David Schwartz, tmayeur@…, hwcdl@…, Katie Vance
Blocked By: Blocking:

Description

Testing Type: Memory leaks

Widget: dijit.Dialog

Description: on IE 8 every time a new Dialog is created, memory is reserved but when the dialog is closed the memory is not released

Steps to reproduce:
1.- Open IE 8 on the /dijit/tests/test_Dialog.html sample page
2.- Open the process explorer tool to monitor the memory used by IE 8
3.- Create/show a new dialog using the buttons on the page (Show dialog, show 2 dialogs, etc)
4.- Delete/close the dialog created, using the "close" button in the dialog.
5.- Repeat the last 2 instructions several times.

Actual results: As you can see on the process explorer tool(image1 attached), every time that a new dialog is shown, new memory is reserved, but when the dialog is closed, the memory is not released, it causes a constantly increase in memory usage by the test page.

Expected results: If new memory is reserved when a dialog is shown, the memory should be released when the dialog is closed.

Note: it is necessary to back-port this issue when fixed to previous versions of IBM dojo Toolkit (at least 1.4.3)

Attachments (2)

image1.jpg (169.3 KB) - added by Roberto Mosqueda 9 years ago.
Dialog_leak.patch (871 bytes) - added by Katie Vance 9 years ago.
Patch to cleanup subscription on destroy

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by Roberto Mosqueda

Attachment: image1.jpg added

comment:1 Changed 9 years ago by bill

Cc: Nathan Toone removed
Summary: on IE 8 every time a new Dialog is created, memory is reserved but when the dialog is closed the memory is not releaseDilaog: initially opening a Dialog increases memory usage, and closing it doesn't release memory (IE8)

Your description confused creating a Dialog with opening it for the first time. Not sure if there's a bug here, since opening the Dialog for the first time does some initialization work.

comment:2 Changed 9 years ago by bill

Summary: Dilaog: initially opening a Dialog increases memory usage, and closing it doesn't release memory (IE8)Dialog: initially opening a Dialog increases memory usage, and closing it doesn't release memory (IE8)

And also, note that closing a Dialog is not the same thing as deleting (aka destroying) it. The dialog still exists, ready to be reopened at a later time.

comment:3 Changed 9 years ago by Katie Vance

Cc: Katie Vance added
Owner: set to bill
Summary: Dialog: initially opening a Dialog increases memory usage, and closing it doesn't release memory (IE8)Subscription in Dialog._setup causes memory leak

There is an initial increase in memory the very first time a dialog is shown and that is because of some initialization that needs to be done. However, continuing to show/hide the dialogs does not increase memory usage. The initialization taking up memory is not proof of a memory leak.

Instead to prove a leak you must create and destroy dialogs repeatedly. After creating, showing, hiding and destroying 1000+ dialogs it's obvious there is a leak. (I used chrome) There is a subscription that is not cleaned up in the _setup method. Cleaning the subscription fixes the issue.

Changed 9 years ago by Katie Vance

Attachment: Dialog_leak.patch added

Patch to cleanup subscription on destroy

comment:4 Changed 9 years ago by bill

Milestone: tbd1.6

Thanks, I'll check that in.

comment:5 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [22745]) Unsubscribe() on destroy() to avoid leak, fixes #11587 !strict. Patch from Katie Vance (IBM, CCLA), thanks! Also combined unsubscribe() and destroy() (getting rid of the unsubscribe() method).

comment:6 Changed 9 years ago by Katie Vance

Bill, can you backport this to the 1.5 branch?

comment:7 Changed 9 years ago by bill

Milestone: 1.61.5.1
Resolution: fixed
Status: closedreopened

OK sure.

comment:8 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [22808]) Backport Dialog memory leak fix to 1.5 branch, fixes #11587 !strict.

Note: See TracTickets for help on using tickets.