Closed Bug 1230027 Opened 6 years ago Closed 6 years ago

Intermittent browser_addonPerformanceAlerts.js,browser_addonPerformanceAlerts_2.js | 1. The real listener was triggered, | 1. The universal listener was triggered, | 1. jank is at least 300ms (0ms)

Categories

(Toolkit :: Performance Monitoring, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: philor, Assigned: sjakthol)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Summary: Intermittent browser_addonPerformanceAlerts.js | 1. The real listener was triggered, | 1. The universal listener was triggered, | 1. jank is at least 300ms (0ms) → Intermittent browser_addonPerformanceAlerts.js,browser_addonPerformanceAlerts_2.js | 1. The real listener was triggered, | 1. The universal listener was triggered, | 1. jank is at least 300ms (0ms)
sendAsyncMessage should be keeping a reference to its argument and keeping it alive. Do you know why it doesn't?
No idea. I really don't know what's going on behind the scenes but obviously the function the CPOW points to in the content process doesn't stay alive long enough since it causes dead CPOW errors when called.
(In reply to :Gavin Sharp [email: gavin@gavinsharp.com] from comment #10)
> sendAsyncMessage should be keeping a reference to its argument and keeping
> it alive. Do you know why it doesn't?

This argument gets used as a CPOW on the other side. I expect that making it the responsibility of the sending message manager itself to keep that object alive wouldn't make sense - it has no way of knowing whether the other process is hanging on to the CPOW on the other side, so it doesn't know when the object can be GC'd on its own side. Whereas the consumer of sendAsyncMessage has a much better chance of knowing when to remove stale bits of data, or when to keep hanging on to them, like in this patch.
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
(In reply to :Gijs Kruitbosch from comment #14)
> This argument gets used as a CPOW on the other side. I expect that making it
> the responsibility of the sending message manager itself to keep that object
> alive wouldn't make sense

Ah yeah, that makes sense!
Comment on attachment 8701351 [details]
MozReview Request: Bug 1230027 - Stop burnCPOWInSandbox from being GC'd during tests. r?yoric

https://reviewboard.mozilla.org/r/28943/#review26145

Ah, thanks, this was puzzling me a lot.
Attachment #8701351 - Flags: review?(dteller) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dc0ca8d57b17
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
(In reply to OrangeFactor Robot from comment #21)
> 39 automation job failures were associated with this bug in the last 7 days.
> 
> Repository breakdown:
> * mozilla-inbound: 19
> * fx-team: 7
> * b2g-inbound: 6
> * try: 4
> * mozilla-central: 3
> 
> Platform breakdown:
> * linux64: 29
> * linux32: 10
> 
> For more details, see:
> https://brasstacks.mozilla.com/orangefactor/
> ?display=Bug&bugid=1230027&startday=2016-01-04&endday=2016-01-10&tree=all

Some of the last items in here (that got starred to this bug) look like they need to get their own bug:

https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=19512902
https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-central&job_id=3044328
https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=19440067

Yoric, can you investigate those in a separate bug?
Flags: needinfo?(dteller)
Good call. Filed as bug 1238520.
Flags: needinfo?(dteller)
You need to log in before you can comment on or make changes to this bug.