Remove JS_ClearScope calls on windows

VERIFIED FIXED in Firefox 13

Status

()

defect
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: gal, Assigned: peterv)

Tracking

(Blocks 1 bug)

unspecified
mozilla13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Reporter

Description

8 years ago
JS_ClearScope is incompatible with modern JS language specifications that our JITs optimize against. We should remove it.
Reporter

Comment 1

8 years ago
Posted patch patch (obsolete) — Splinter Review
Work in progress. Incomplete. Not ready for review.
Assignee: general → gal
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.
Blocks: 651030
Reporter

Comment 3

8 years ago
peterv is working on this
Assignee: gal → peterv
Assignee

Updated

8 years ago
Depends on: 658632
Assignee

Updated

8 years ago
Depends on: 658636
Assignee

Updated

8 years ago
Depends on: 658637
Assignee

Updated

8 years ago
Depends on: 659580
Assignee

Updated

8 years ago
Depends on: 659581
Assignee

Comment 4

8 years ago
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.
our hero!
Assignee

Updated

8 years ago
Depends on: 662243
Can't wait.

/be
Assignee

Comment 7

8 years ago
(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

Comment 9

8 years ago
If you'd like me to fuzz this before it lands, attach an updated patch and let me know :)

Updated

8 years ago
Blocks: 664797
Assignee

Comment 10

8 years ago
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.
Blocks: 680456
No longer blocks: 680456
Blocks: 680456
Assignee

Comment 11

8 years ago
Still debugging a leak.
Assignee

Updated

8 years ago
Depends on: 682539
Blocks: 635548
Assignee

Comment 12

8 years ago
Posted patch v2 (obsolete) — Splinter Review
Attachment #515422 - Attachment is obsolete: true
Attachment #557951 - Flags: review?(mrbkap)
Comment on attachment 557951 [details] [diff] [review]
v2

This looks awesome.
Attachment #557951 - Flags: review?(mrbkap) → review+
It looks like there's been an r+'d patch here for about a month. Is that patch ready to land?
Assignee

Comment 15

8 years ago
Unfortunately no, there's another mochitest leak that I'm tracking down.
Assignee

Updated

8 years ago
Depends on: 696817
Assignee

Updated

8 years ago
Depends on: 699796
Assignee

Updated

8 years ago
Depends on: 699807
Blocks: 711695
Posted patch v2.1 (obsolete) — Splinter Review
Merged, still leaks.
No longer blocks: 721284
Blocks: 715149
Assignee

Updated

7 years ago
Depends on: 731173
Assignee

Comment 17

7 years ago
Posted patch v2.1Splinter Review
Updated to trunk, needs leak fix from bug 731173.
Attachment #557951 - Attachment is obsolete: true
Attachment #588405 - Attachment is obsolete: true
Attachment #601285 - Flags: review+

Updated

7 years ago
Summary: Remove JS_ClearScope → Remove JS_ClearScope calls on windows
https://hg.mozilla.org/mozilla-central/rev/491ceed82be3
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Status: RESOLVED → VERIFIED

Updated

7 years ago
Depends on: 837011
You need to log in before you can comment on or make changes to this bug.