Closed Bug 773610 Opened 8 years ago Closed 5 years ago

Fix consumers that AddRef XPCWrappedJS off-main-thread

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bholley, Assigned: bholley)

References

Details

Split off from bug 771074.
Fixed various consumers and pushed to try with fatal asserts. Let's see what our test suite hits:

https://tbpl.mozilla.org/?tree=Try&rev=7d5f0d9086ab
Fixed more and pushed again:
https://tbpl.mozilla.org/?tree=Try&rev=c3b0a1617a2a
Note that my patches are in a github branch here:  https://github.com/bholley/mozilla-central/commits/wrappedjs_dethread
(In reply to Bobby Holley (:bholley) from comment #7)
> https://tbpl.mozilla.org/?tree=Try&rev=0d1e656367ce

Nice, linux is green! It's pretty likely that there are going to be some platform-specific failures here, so let's check those now:

https://tbpl.mozilla.org/?tree=Try&rev=b753c766c5ef
Blocks: 844088
Blocks: 843923
Blocks: 839535
This is hitting an out-of-bounds nsTArray on windows that MOZ_ASSERTs before we get a stack. I replaced it with an NS_ASSERTION, which should hopefully dump a stack:

https://tbpl.mozilla.org/?tree=Try&rev=1bed20aef921
Ugh, the stacks here are being supremely unhelpful, and this doesn't reproduce locally for me.

I'm pretty sure it's related to one of the three patches that do something with nsTArray, so I'm pushing those three separately to try to see if we can get some more data:

https://tbpl.mozilla.org/?tree=Try&rev=1ff159e0aae5
https://tbpl.mozilla.org/?tree=Try&rev=e8bd699b2ce6
https://tbpl.mozilla.org/?tree=Try&rev=8193765f4429
I'm starting to think that the issue here might just be that nsTArray asserts against out-of-bounds access but nsCOMArray doesn't. Let's test that theory:

https://tbpl.mozilla.org/?tree=Try&rev=2795f3a43e11
Depends on: 850054
Hm, so all three of the suspected patches on comment 10 went green on their own. I'm going to divide these up, start getting them reviewed, and focus on landing them individually to sort out the wackiness and decrease regression potential.
Depends on: 850245
Depends on: 850246
Depends on: 850247
Depends on: 850249
Depends on: 850251
Depends on: 850252
Depends on: 850255
Depends on: 850253
Moving deps up a few levels.
No longer blocks: 839535, 843923
No longer blocks: 844088
All the blocking bugs here have been fixed, but there are still some places that need to be fixed up.  See bug 770840 comment 4.
Just making sure I don't lose track of this.
Flags: needinfo?(josh)
Depends on: 869893
Depends on: 869928
Depends on: 870782
Depends on: 870864
Depends on: 870916
Depends on: 873615
Depends on: 870887
Depends on: 871125
Depends on: 882125
Depends on: 882424
Depends on: 882200
Depends on: 882196
Depends on: 882637
Depends on: 882328
Depends on: 883495
Depends on: 884200
Depends on: 884792
Flags: needinfo?(josh)
Depends on: 885292
Depends on: 887191
Depends on: 889885
Depends on: 892340
Depends on: 899453
We assert against this now.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.