Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#17048 closed defect (fixed)

dojo/aspect leaks memory when removed handles are retained elsewhere

Reported by: Colin Snover Owned by: Colin Snover
Priority: blocker Milestone: 1.7.5
Component: Core Version: 1.8.3
Keywords: Cc: Brian Arnold, Kris Zyp
Blocked By: Blocking:

Description (last modified by Colin Snover)

Consider the following:

(function () {
  var a = createLargeObject(),
      b = function () {};

  window.handle = aspect.after(a, 'foo', b);
  window.handle.remove();
})();

Even though the handle has been removed, a and b continue to be retained through the global reference to the handle object itself as dispatcher and advice, which means that any time you use dojo/aspect without making sure you delete all references to a handle—even if the handle is “removed”—you are leaking potentially huge numbers of objects.

The solution is to simply make sure that all references retained through closures on the returned handle object are nulled out when remove is called. Users might still leak the handle, but at least it won’t leak all their stuff too.

Attachments (1)

aspect.js.patch (950 bytes) - added by Kris Zyp 7 years ago.
test case that regresses with changes

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by Colin Snover

Description: modified (diff)

comment:2 Changed 7 years ago by Colin Snover

Status: newassigned

comment:3 Changed 7 years ago by Colin Snover

Resolution: fixed
Status: assignedclosed

In [31298]:

Clear references within dojo/aspect when handles are removed to avoid
over-retention of objects when a user removes a handle and then does
not dereference the handle. Fixes #17048.

Also, correct [30256] to allow remove functions to work correctly
when their context is changed, refs #16513.

!strict due to "use strict" pragma

comment:4 Changed 7 years ago by Colin Snover

In [31299]:

Clear references within dojo/aspect when handles are removed to avoid
over-retention of objects when a user removes a handle and then does
not dereference the handle. Fixes #17048.

Prevent multiple remove() calls on an aspect signal from corrupting the aspect chain, fixes #16513

!strict due to "use strict" pragma

Backport to 1.8

comment:5 Changed 7 years ago by Colin Snover

In [31300]:

Clear references within dojo/aspect when handles are removed to avoid
over-retention of objects when a user removes a handle and then does
not dereference the handle. Fixes #17048.

Prevent multiple remove() calls on an aspect signal from corrupting the aspect chain, fixes #16513

!strict due to "use strict" pragma

Backport to 1.7

comment:6 Changed 7 years ago by Colin Snover

Milestone: 1.8.51.7.5

comment:7 Changed 7 years ago by Kris Zyp

Resolution: fixed
Status: closedreopened

Changed 7 years ago by Kris Zyp

Attachment: aspect.js.patch added

test case that regresses with changes

comment:8 Changed 7 years ago by Colin Snover

Resolution: fixed
Status: reopenedclosed

In [31308]:

Remove incorrect method call. Fixes #17048. !strict due to strict mode

comment:9 Changed 7 years ago by Colin Snover

In [31309]:

Remove incorrect method call. Fixes #17048. !strict due to strict mode
Backport to 1.8

comment:10 Changed 7 years ago by Colin Snover

In [31310]:

Remove incorrect method call. Fixes #17048. !strict due to strict mode
Backport to 1.7

Note: See TracTickets for help on using tickets.