792208 - Can't access dead object error in the CraigslistFusion addons

Status: RESOLVED INVALID

(WebExtensions :: General, defect)

Description

[Nam H]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120907231657

Steps to reproduce:

1. Using Craigslist Fusion addon, go to Craigslist, and search for something.  
2. Search again for something else.
3. Click on the map (arrow) icon next to the title of 1 item in the result list

The error is shown as "TypeError: can't access dead object".





Actual results:

On the click of item # above, there is code beneath calls document.getElementById.  The return value is supposed to be a DIV that was created earlier.  This DIV was  hidden and should be shown after clicking on that arrow.  However, the error message was shown.  It appears that if the DIV was created and hidden (and maybe empty?), it got collected by memory.  However, I don't know for sure.  Maybe there is more to that.


Expected results:

A map should show up showing the location of the resulted item.
This problem doesn't seem to happen in the exact same script but used inside GreaseMonkey

Comment 1

[Nam H]

Correction/Update:

The previous text is not accurate:

"On the click of item # above, there is code beneath calls document.getElementById.  The return value is supposed to be a DIV that was created earlier.  This DIV was  hidden and should be shown after clicking on that arrow."

The div does not cause the error.  It was the div.style.zIndex was causing issue.  I comment that line out, but left other lines with div.style.<something>, no problem.  So it's not the style either.  It is the zIndex that is causing the problem.

Comment 2

[Nam H]

Basically, I found that setting the zIndex value using JS would fail after initial loading code.  DIV doesn't need to be invisible before that.

Comment 3

[Nam H]

Further looking into this, I found a few issues.

1. First, new page/dom nodes with old event listeners

after clicking the search button on the Craigslist page again, the page is reloaded, and I assume they are new DOM nodes.  If they're not, they should be "cleaned up" properly before reuse as part of any optimization.  I think this is Firefox issue.  The reason I know that the new listing items fires old event handlers since I debugged and it shows the old event handler message.

I understand that there MAYBE a requirement to remove all registered events.  Even if I don't remove the registered events, they should not be fired on new nodes (probably reused somehow).  Also, why the page reload, and the old DOM nodes are removed, but the events are not cleaned up?

2.  If I have to remove all event handlers, there is a big problem here.  If I keep a list of nodes objects and event handlers to remove in my code (when page unload), then that is a memory leak itself (keeping handles to nodes that may longer be needed before the page is unload).  For example, if I show a list of items, and have event listeners for them.  Next, I remove the items from the page dynamically by removing the containing DIV.  If I don't clean up the event listeners to those items at that point, and wait until the page unload, then those items are referenced, and causes memory leak.  If they're not referenced (by the memory clean up mentioned above), then somehow the event listeners are dangling and causing referencing dead object issues I am currently experienced.  Sure, someone may say I need to keep track of all event listeners and remove them as soon as possible.  This maybe possible, but is very tedious and easily missed by many developers.  There should be a global solution to this.

Comment 4

[Nam H]

A related tracker: https://bugzilla.mozilla.org/show_bug.cgi?id=241518

Comment 5

[Nam H]

Please note that with version 8.5 of the add-on (CraigslistFusion), this error doesn't pop up since I trap it as a temporary work around.  To test it, please use version 8.4.

Comment 6

[Nam H]

Close this issue due to actual memory leak found that referencing old objects from previous page.
(sorry all for this tracker, Firefox 15 really rocks on this).

Comment 7

[Kris Maglione [:kmag]]

Sorry, are you saying Craiglist Fusion is not causing these errors? Are you the author of the add-on in question (it's generally best to make that clear when you file these kinds of bugs)?

Comment 8

[Nam H]

Kris, I am the author.  The memory leak is inside the addon (Craigslist Fusion).  The events generated above were from the new page's DOM nodes (not the old page's nodes).  However, the listener/handler calls an object that references old data.  I have made correction to the add-ons to remove the objects that references old data in the version 8.5.1.

Comment 9

[Nam H]

Kris, I closed the tracker as invalid because this is not a Firefox bug.  Although the title is clear above, but I suppose this is a Firefox bug database.