Closed Bug 736186 (LA-coknown-research-) Opened 12 years ago Closed 12 years ago

CoKnown add-on creates zombie compartments

Categories

(WebExtensions :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kmag, Unassigned)

References

()

Details

(Whiteboard: [MemShrink:P3])

I've only confirmed that this issue occurs in the version currently pending review. I have no reason to doubt that it also occurs in the current public release as the changes were rather minor.

To reproduce, I installed CoKnown in a relatively clean profile:

1) Log in (I have test credentials, but I don't think I can make them public)
2) Visit http://www.google.com/
3) Select a project
4) Click Clip page
5) Click the page
6) Click Sage
7) Fill in random values and press Save
8) Close tab and minimize memory. Compartment remains.

It is possible that some or all of these steps may be omitted.

Not CCing author, as he does not appear to have a Bugzilla account, but he has been notified via AMO.
Kris:  While this will certainly be triaged as a MemShrink:P3, we generally leave MemShrink bugs untriaged until the meeting so that people attending the meeting can see the bug and choose to CC themselves if they want :)
Whiteboard: [MemShrink:P3] → [MemShrink]
Eh, I just follow the pack. I'll update my template.
Whiteboard: [MemShrink] → [MemShrink:P3]
Developer chased via info request.
Component: General → Add-ons
Product: Core → Tech Evangelism
Sent a new message to the developer. The add-on will be downgraded next week if no progress is shown.
Hi all, 
we are diligently attempting to resolve the Zombie compartment issue. This is not easy! It appears jQuery is holding onto references to pages which we can't flush. Since these references are still in JS, Firefox is picking them up and won't release the compartments. If we null out everything in jQuery we hang jQuery and the Toolbar.  Any suggestions you have on how to get jQuery to run it's garbage cleanup without hanging would be greatly appreciated.
I talked with Eric via email and made some suggestions on how to resolve this.
Hi Jorge, as I said in my reply email to you, we believe we have this issue resolved. We've tested it and all compartments clear. (takes 6-10 refreshes of about:memory?verbose, but that seems to be the way that feature works)  The latest Toolbar vr. 2.0.24 has been in the cue since 4/22 awaiting review. 
We also addressed all other Kris's suggested changes in this version. 

Thank you
Hi guys, 
have we slipped through the cracks? 
We fixed this issue on April 22nd. Version 2.0.24

We don't seem to be moving in the review queue.
We're getting desperate since the version(2.0.15) we have up doesn't have any of the usability/help updates in it. 

Thank you,
Eric
Add-on reviews are taking longer than usual these days. We are very short in contributions.

I reviewed your update and the leak hasn't been fixed. I can still reproduce it following the steps in comment #0, and waiting for a while and clicking on the Minimize button a dozen times or so didn't help.
Hi Jorge,
did you have any other extensions running while you did this? 
We've seen this happen when Firebug is running. It doesn't do it when Firebug is disabled.
Sorry for the delay.

I can reproduce the zombie compartment reliably with all other add-ons disabled.
Eric, we need you to address this issue as soon as possible. If we don't see any activity towards fixing this problem, we will downgrade your add-on to preliminarily approved in order to minimize future user impact.
We are working **** this. We have resolved the Zombie issue, however resolving that caused another issue when saving page clippings from Linux and Windows. Works great on Mac. Should have a new version in the queue early next week.
Eric, what's the status of your update?
Is there any progress on a fix for this? It was reported three and a half months ago. If it goes unfixed for much longer, we're going to have to demote it to full review.

Thanks.
Hi Jorge and Kris, we are making progress on this. It's requiring almost a full rewrite to make this happen. Currently we have a working copy for Mac. However, Linux and windows handle variables in a different fashion and subsequently the solution requires yet another rewrite. All of this is extremely time intensive. Thank you.
What do you mean, Linux and Windows handle variables differently? They should be exactly the same.
Hi Kris,
Mac's pass the actual variable contents. While Linux and PC's pass a pointer. So when we destroy the object to get rid of all traces of a url held internally within jQuery the variable's content is gone. Works great on Mac since the variable contents are passed. Throws javascript null errors on PC's and Linux. 
To make a very long story short. jQuery holds onto references to urls within it's base code (not accessible by any commands without hacking jQuery). These urls are in turn are picked up by Firefox. Then you get a Zombie compartment. Since there's no method for destroying either references in Firefox or jQuery, the only solution is to destroy the entire object. This works on Mac since it passes the variable contents themselves, however, PC's and Linux pass a pointer to where the information is stored -- Which we just destroyed to get rid of the compartment. Catch 22. The current rewrite is passing the variable contents prior to being destroyed to a global variable, which we can then use and destroy when we're done with it. So there you go, we now know what's happening, we just have to rewrite everything yet again to make it happen.
Sorry... what context are you talking about here? JavaScript objects are passed by reference everywhere. The only times they're passed by value is when scripts make explicit copies, or you're passing through some structured clone barrier, like worker messaging. But those work the same regardless of OS.
Turns out we've had this fixed for about 3 weeks. We changed the code to recreate the croptool object. The result is that jQuery is wiped clean and Firefox doesn't pick up the urls from jQuery's internal memory. 

The 3 week delay seems to involve Firefox syncing between our developer's computers (Mac, Linux and 2 PCs) and resulted in the CoKnown toolbar throwing JavaScript null errors continuously. This is isolated to only his machines (Firefox synced) and does not effect anyone else. Toolbar has been tested on multiple other computers and works great. He cannot get any of his computers to run the toolbar any longer. Removing and reinstalling Firefox does not seem to resolve the issue. 

Is there a way for him to wipe clean his Firefox syncing account somewhere? It seems something is corrupted in it. Or to delete the sync account entirely and start a new account? 

Thank you,
Eric
Forgot to add that version 2.0.25 has been uploaded and is awaiting review. 
Thanx again.
I can't reproduce this with the latest release.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: Add-ons → General
Product: Tech Evangelism → WebExtensions
You need to log in before you can comment on or make changes to this bug.