Skip to content

Instantly share code, notes, and snippets.

@kriszyp
Created November 12, 2013 22:05
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save kriszyp/7439619 to your computer and use it in GitHub Desktop.
Save kriszyp/7439619 to your computer and use it in GitHub Desktop.
Whether or not to auto-own() widget aspects
Dylan was asking me about the own()'ing of aspects, so I thought I would try to write out the issues in case it would help. The issue associated with not own()ing the function (https://github.com/dojo/dijit/blob/1.9.1/_WidgetBase.js#L940), is that if a reference to the widget or the handle is retained after destruction, the function is still referenced just as it was when it was live. What has made this a debatable issue though, is that if a widget is referenced after it is destroyed, is that really a valid use case? Is leaving such a reference itself a memory leak, and should be fixed, rather than trying to make memory leaks be smaller leaks? The downside of own()ing all aspect handles is that own()ing has a cost itself, each owned reference takes up a slot in an array, so by own()ing aspects, we are effectively punishing the vast majority of widgets do not ever end up with a reference to a destroyed widget.
It might help to look at a few different scenarios:
A single widget: when a widget listens/aspects to one of its own widgets, it is circular JS reference that is easily collected. Owning handles to these aspects cost extra memory with no gain.
Two widget scenario, where widget A aspects a method on widget B (and A follows best practice and own()s the handle), and then...
widget A and B are destroyed: no memory leak, everything is in a cycle that can be collected
widget A is destroyed (and B is still alive): widget A own()ed the handle, so it is cleaned up (in both cases where A is destroyed, B's ownership merely costs memory with no gain).
widget B is destroyed (and A is still alive): This is the situation that can introduce a memory leak, since A doesn't own the handle.
However, this last case is seems somewhat unorthodox. Generally, when one widget listens to another it often implies a dependency. A is depending on B, and just like with modules, B may be able to exist without A, but A can't exist without B. If A has another reference to B (to make calls to B), then B's own()ership of a handle, and auto-removal on destruction of B, will do nothing to actually make B garbage collectable if A still has another reference to it. In most cases where A listens to events on B it is part of a hierarchy, B is destroyed only when A is destroyed.
Still though it is possible to imagine a scenario where this type of aspecting would occur and cause a problem. But I think the question of whether this should be auto-own()ed is centered around how exceptional this case is, and whether it should be explicitly handled by the developer in the rare case that such a soft-dependency actually exists, so as to avoid the overhead of always own()-ing handles, or if we should always own() aspects, accepting the overhead for the sake of not making developers worry about this edge case.
Another concern with not own()-ing aspect handles, is that we do always have widgets own() their node event listening handles, and so there is an inconsistency. However, historically, there is more of a concern in regards to own()ing node listeners because in old IE, each of these listeners are referenced through a global array, and in order for these to be collected (prior to page reload), the handles must be removed, their removal in old IE is essential to the collection of destroyed widgets. For modern apps, one could alternately argue for removing the own()ing of node handles to achieve consistency.
I didn't write https://github.com/dojo/dijit/blob/1.9.1/_WidgetBase.js, so I don't think I can authoritatively answer why we chose to not own (and maybe we will at some point). I am generally biased towards performance over developer convenience, so not own-ing makes sense to me, but I understand the case for own()'ing aspects as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment