JS_ClearScope is incompatible with modern JS language specifications that our JITs optimize against. We should remove it.
Created attachment 515422 [details] [diff] [review] patch Work in progress. Incomplete. Not ready for review.
Andreas, do you still intend to work on this or should we find someone else. I really want it fixed because it keeps biting us. Bug 651030 is the latest.
peterv is working on this
With the fixes for the 5 dependent bugs this is looking close, I think there's only one remaining leak in mochitest-a11y. Still tracking that one down.
Can't wait. /be
(In reply to comment #6) > Can't wait. Your review of the patch in bug 659581 will make it happen sooner :-).
(In reply to comment #7) > (In reply to comment #6) > > Can't wait. > > Your review of the patch in bug 659581 will make it happen sooner :-). Done! Sorry for the hold-up. sfink is good for jsd reviews too, I think. timeless has moved on from Nokia and Mozilla stuff, FYI. /be
If you'd like me to fuzz this before it lands, attach an updated patch and let me know :)
There are a number of JS_ClearScope callers, and it's unclear to me if we can now remove them all. http://mxr.mozilla.org/mozilla-central/search?string=ClearScope&find=&findi=&filter=xpconnect|ipc|dom&hitlimit=&tree=mozilla-central Storage code uses it to remove all properties from a JS object, we'll need to do something else here. What should it do instead? I think the XPConnect module loader uses it at shutdown to avoid leaks, I haven't tried to remove that one yet. There might be more leaks to fix as a result of removing it :-/. We still need to make sure that removing the code that clears watchpoints is ok (bug 38959). The code that invalidates the "global scope polluter" looks like it was added to fix a leak (bug 256932), so that should be fine.
Still debugging a leak.
Created attachment 557951 [details] [diff] [review] v2
Comment on attachment 557951 [details] [diff] [review] v2 This looks awesome.
It looks like there's been an r+'d patch here for about a month. Is that patch ready to land?
Unfortunately no, there's another mochitest leak that I'm tracking down.
Created attachment 601285 [details] [diff] [review] v2.1 Updated to trunk, needs leak fix from bug 731173.