Crash in [@ mozilla::net::IOServiceProxyCallback::OnProxyAvailable]

RESOLVED FIXED in Firefox 67

Status

()

defect
P1
critical
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: marcia, Assigned: ckerschb)

Tracking

({crash, regression})

Trunk
mozilla67
Unspecified
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 fixed)

Details

(Whiteboard: [domsecurity-active], crash signature)

Attachments

(1 attachment)

This bug is for crash report bp-cd1a6e21-cc5a-4e3e-b9ba-cae9f0190306.

Seen while looking at crash stats on nightly - crashes started in 67 using 20190225014816: https://bit.ly/2VBpdlq. 7 crashes/6 installs so far, all on Windows.

Code was touched in the 67 timeframe in Bug 1528677. There are crashes in 65 and 66, but there isn't the small spike that is seen on 67.

ni on ckerschb for some insight.

Top 10 frames of crashing thread:

0 xul.dll nsresult mozilla::net::IOServiceProxyCallback::OnProxyAvailable netwerk/base/nsIOService.cpp:1723
1 xul.dll static nsresult mozilla::net::nsAsyncResolveRequest::DoCallback::<unnamed-tag>::operator netwerk/base/nsProtocolProxyService.cpp:358
2 xul.dll xul.dll@0x197c525 
3 xul.dll nsresult mozilla::net::nsAsyncResolveRequest::AsyncApplyFilters::Finish netwerk/base/nsProtocolProxyService.cpp:589
4 xul.dll nsresult mozilla::net::nsAsyncResolveRequest::AsyncApplyFilters::ProcessNextFilter netwerk/base/nsProtocolProxyService.cpp:500
5 xul.dll nsresult mozilla::net::nsAsyncResolveRequest::AsyncApplyFilters::AsyncProcess netwerk/base/nsProtocolProxyService.cpp:476
6 xul.dll void mozilla::net::nsAsyncResolveRequest::DoCallback netwerk/base/nsProtocolProxyService.cpp:367
7 xul.dll nsresult mozilla::net::ExecuteCallback::Run netwerk/base/nsPACMan.cpp:119
8 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1166
9 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:482

Flags: needinfo?(ckerschb)

(In reply to Marcia Knous [:marcia - needinfo? me] from comment #0)

Code was touched in the 67 timeframe in Bug 1528677. There are crashes in 65 and 66, but there isn't the small spike that is seen on 67.

Mhm, so apparently there must still be a codepath that allows to create a channel without a loadinfo. Maybe it's a (de)serialization problem somewhere. It's odd that it's only affecting windows, because channels and loadinfo shouldn't be platform specific.

I would be too curious why we would end up without a loadinfo.

Is there any way we can get STRs?

(Keeping ni? for further investigation).

Flags: needinfo?(mozillamarcia.knous)

I will keep an eye out in crash stats for more information - so far there are no comments and really nothing to go in terms of URLs.

Flags: needinfo?(mozillamarcia.knous)

(In reply to Marcia Knous [:marcia - needinfo? me] from comment #2)

I will keep an eye out in crash stats for more information - so far there are no comments and really nothing to go in terms of URLs.

OK - thanks. Please ni? me in case we have STRs. I can then look at the problem right away - thanks!

Flags: needinfo?(ckerschb)

No further crashes in nightly since the ones from 3-5 and 3-6.

Valentin, Honza, this bug makes me slightly nervous after all. There shouldn't be any channels that do not have a Loadinfo. I know we have some channels that are not nsIChannels. Since crashes are super rare it might be one of those channels - maybe view:source or something.

Anyway, any suggestions on how I could possibly figure out what's going on? Should we add some kind of assert back at that code location that spits out some information? Any other suggestions?

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(honzab.moz)

My suggestion would be to add a release assert aLoadInfo is non-null to all SetLoadInfo implementations.
I took a look at some of the crashes, but unfortunately I couldn't identify what kind of channels they were.
One has the URL about:newtab, another about:sessionrestore, and one was google.com/search.
Not sure how useful that is.

Flags: needinfo?(valentin.gosu)

(In reply to Valentin Gosu [:valentin] from comment #6)

My suggestion would be to add a release assert aLoadInfo is non-null to all SetLoadInfo implementations.
I took a look at some of the crashes, but unfortunately I couldn't identify what kind of channels they were.
One has the URL about:newtab, another about:sessionrestore, and one was google.com/search.
Not sure how useful that is.

Yeah, that sounds reasonable to me - I'll do that - thanks!

Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [domsecurity-active]

Valentin suggested a good next step - clearing ni? for Honza.

Flags: needinfo?(honzab.moz)

I did 'Preview Landing' in Lando and it showed me 'Landing Queued' but the patch never landed - until I figure out what the problem was I am requesting 'checkin-needed' for this one.

Keywords: checkin-needed
Attachment #9050604 - Attachment description: Summary: Bug 1533159: Add MOZ_RELEASE_ASSERT to all SetLoadInfo implementations to ensure loadinfo is never null. r=valenin → Bug 1533159: Add MOZ_RELEASE_ASSERT to all SetLoadInfo implementations to ensure loadinfo is never null. r=valenin

Comment 11

2 months ago

Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/autoland/rev/d8b7c29dcee0
Add MOZ_RELEASE_ASSERT to all SetLoadInfo implementations to ensure loadinfo is never null. r=valentin

Keywords: checkin-needed

Comment 12

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Updated

2 months ago
Depends on: 1537883
Depends on: 1541161
No longer depends on: 1541161
Regressions: 1541161
You need to log in before you can comment on or make changes to this bug.