Fix consumers that AddRef XPCWrappedJS off-main-thread

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
6 years ago
3 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

6 years ago
Split off from bug 771074.
(Assignee)

Comment 1

6 years ago
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
(Assignee)

Comment 2

6 years ago
Fixed more and pushed again:
https://tbpl.mozilla.org/?tree=Try&rev=c3b0a1617a2a
(Assignee)

Comment 5

6 years ago
Note that my patches are in a github branch here:  https://github.com/bholley/mozilla-central/commits/wrappedjs_dethread
(Assignee)

Comment 8

6 years ago
(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
(Assignee)

Updated

6 years ago
Blocks: 839535
(Assignee)

Comment 9

6 years ago
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
(Assignee)

Comment 10

6 years ago
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
(Assignee)

Comment 11

6 years ago
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
(Assignee)

Comment 12

6 years ago
@#$% warnings-as-errors - https://tbpl.mozilla.org/?tree=Try&rev=4f50a949b7df
(Assignee)

Updated

6 years ago
Depends on: 850054
(Assignee)

Comment 13

6 years ago
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.
(Assignee)

Updated

6 years ago
Depends on: 850245
(Assignee)

Updated

6 years ago
Depends on: 850246
(Assignee)

Updated

6 years ago
Depends on: 850247
(Assignee)

Updated

6 years ago
Depends on: 850249
(Assignee)

Updated

6 years ago
Depends on: 850251
(Assignee)

Updated

6 years ago
Depends on: 850252
(Assignee)

Updated

6 years ago
Depends on: 850255
(Assignee)

Updated

6 years ago
Depends on: 850253
(Assignee)

Comment 14

6 years ago
Moving deps up a few levels.
No longer blocks: 839535, 843923
No longer blocks: 844088
Blocks: 843923
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)

Updated

6 years ago
Depends on: 869893

Updated

6 years ago
Depends on: 869928

Updated

6 years ago
Depends on: 870782

Updated

6 years ago
Depends on: 870864

Updated

6 years ago
Depends on: 870916

Updated

6 years ago
Depends on: 873615

Updated

6 years ago
Depends on: 870887

Updated

6 years ago
Depends on: 871125

Updated

6 years ago
Depends on: 882125

Updated

6 years ago
Depends on: 882424

Updated

6 years ago
Depends on: 882200

Updated

6 years ago
Depends on: 882196

Updated

6 years ago
Depends on: 882637

Updated

6 years ago
Depends on: 882328

Updated

6 years ago
Depends on: 883495

Updated

6 years ago
Depends on: 884200

Updated

6 years ago
Depends on: 884792

Updated

6 years ago
Flags: needinfo?(josh)

Updated

6 years ago
Depends on: 885292

Updated

6 years ago
Depends on: 887191

Updated

6 years ago
Depends on: 889885

Updated

5 years ago
Depends on: 892340

Updated

5 years ago
Depends on: 899453
(Assignee)

Comment 17

3 years ago
We assert against this now.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.