1342877 - Assertion failure: isMainDocumentChannel, at HttpChannelChild.cpp:2085

Status: NEW

(Core :: Networking: HTTP, defect, P3)

Description

[Carsten Book [:Tomcat]]

found via bughunter and reproduced on latest m-c tinderbox trunk build 

Steps to reproduce:
-> Load http://kinodisco.at.ua/load/podborki_filmov/divergent_1_2_3_4_vse_chasti_po_porjadku/58-1-0-1039
--> Assertion failure 

Assertion failure: isMainDocumentChannel, at c:/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/src/netwerk/protocol/http/HttpChannelChild.cpp:2085
#01: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x440ef0]
#02: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x45b843]
#03: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x45e2f6]
#04: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x45a0b5]
#05: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x5ecf4c]
#06: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x7b398a]
#07: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x55fcb2]
#08: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x5601da]
#09: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x567412]
#10: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x566bf5]
#11: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x1aa949]
#12: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x1b4af9]
#13: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x1b40f9]
#14: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x5668eb]
#15: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x5669ee]
#16: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547f82]
#17: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547f3a]
#18: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547c91]
#19: mozilla_dump_image[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x1b4b33e]
#20: mozilla_dump_image[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x1b9e19c]
#21: workerlz4_maxCompressedSize[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x28b2542]
#22: soundtouch::SoundTouch::operator=[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x566963]
#23: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547f82]
#24: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547f3a]
#25: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x547c91]
#26: workerlz4_maxCompressedSize[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x28b20a8]
#27: workerlz4_maxCompressedSize[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\xul.dll +0x28bb2b0]
#28: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\firefox.exe +0x1584]
#29: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\firefox.exe +0x1340]
#30: ???[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\firefox.exe +0x1a79]
#31: TargetNtUnmapViewOfSection[c:\bughunter\firefox-54.0a1.en-US.win32\firefox\firefox.exe +0x32b8d]
#32: BaseThreadInitThunk[C:\Windows\system32\kernel32.dll +0x4ee1c]
#33: RtlInitializeExceptionChain[C:\Windows\SYSTEM32\ntdll.dll +0x637eb]
#34: RtlInitializeExceptionChain[C:\Windows\SYSTEM32\ntdll.dll +0x637be]

Comment 1

[Carsten Book [:Tomcat]]

Mayhemer, Patrick, could you take a look ? Thanks

Comment 2

[Honza Bambas (:mayhemer)]

Bill, you've added this check in bug 1318506, do you have any idea why it fails here?  This is easily reproducible.

URL of the failing channel is http://sync.bumlam.com/?src=mirs1&s_data=CAIQARj82NXFBWIMaDFza1FvMjBSMHFlogEQFZ_hGP2xEeam6QAlkMgkNw**

Which is a target from http://sync3.adsniper.ru/?src=ss1&s_data=CAEQABj82NXFBVIFpMTQrwZiDGgxc2tRbzIwUjBxZQ** which is already result of a redirect too.

For me it failed with a different stack:

>	xul.dll!mozilla::net::HttpChannelChild::SetEventTarget() Line 2085	C++
 	xul.dll!mozilla::net::HttpChannelChild::ConnectParent(0x00000017) Line 1651	C++
 	xul.dll!mozilla::net::HttpChannelChild::Redirect1Begin(0x00000017, {...}, 0x00000001, {...}, {...}, {...}) Line 1415	C++
 	xul.dll!mozilla::net::Redirect1Event::Run() Line 1296	C++
 	xul.dll!mozilla::net::ChannelEventQueue::RunOrEnqueue(0x06d60710, false) Line 139	C++
 	xul.dll!mozilla::net::HttpChannelChild::RecvRedirect1Begin(0x00000017, {...}, 0x00000001, {...}, {...}, {...}) Line 1321	C++
 	xul.dll!mozilla::net::PHttpChannelChild::OnMessageReceived({...}) Line 897	C++

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I wasn't able to reproduce. Maybe it's serving me different ads or something.

The purpose of the assertion is to ensure that, if the load info doesn't have a document, but it does have an outer window, then we must be loading a top-level document. I can think of two possible reasons for this:
- we're forgetting to set the document in the load info
- we're not setting isMainDocumentChannel on the channel when we should be

It could also be some sort of weird load type that I hadn't thought of.

Can you still reproduce Honza?

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to Bill McCloskey (:billm) from comment #3)
> I wasn't able to reproduce. Maybe it's serving me different ads or something.
> 
> The purpose of the assertion is to ensure that, if the load info doesn't
> have a document, but it does have an outer window, then we must be loading a
> top-level document. I can think of two possible reasons for this:
> - we're forgetting to set the document in the load info
> - we're not setting isMainDocumentChannel on the channel when we should be

That's obvious.  Where are the places we can look for?  Can you provide some more info as author of this code please?

> 
> It could also be some sort of weird load type that I hadn't thought of.
> 
> Can you still reproduce Honza?

I was able to reproduce once, but now I can't too...

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Honza Bambas (:mayhemer) from comment #4)
> (In reply to Bill McCloskey (:billm) from comment #3)
> > I wasn't able to reproduce. Maybe it's serving me different ads or something.
> > 
> > The purpose of the assertion is to ensure that, if the load info doesn't
> > have a document, but it does have an outer window, then we must be loading a
> > top-level document. I can think of two possible reasons for this:
> > - we're forgetting to set the document in the load info
> > - we're not setting isMainDocumentChannel on the channel when we should be
> 
> That's obvious.  Where are the places we can look for?  Can you provide some
> more info as author of this code please?

I'm pretty certain you know more about this code than I do :-). When Patrick reviewed bug 1318506, he asked me to assert that (mLoadFlags & LOAD_DOCUMENT_URI) on the channel. I tried that and the assertion failed on try because of view source channels. So I looked around and found this GetIsMainDocumentChannel thing that seemed more suitable and I used that.

I don't know much about how load flags are set on a channel. The document in the loadinfo seems to come from the loading context, which is a pretty generic thing that we pass around all over the place. I guess it would make sense to audit how these two things are handled inside redirects. I don't know where that happens though.

I'm sorry I'm not more helpful. We can always remove this assertion. However, it's asserting something that seems like it ought to be true, so it would be nice to understand why it fails.

Comment 6

[Honza Bambas (:mayhemer)]

Thanks.  I'm afraid that w/o reliable steps to reproduce or a working test case, we won't be able to move much forward here :(

Comment 7

[Andrew McCreight [:mccr8]]

I'm seeing this assertion on dilbert.com. I can reproduce it fairly reliably by opening dilbert.com in 3 tabs, and reloading the tabs over and over, which I guess would be consistent with it being some particular ad.

I also sometimes get this assertion instead:
Assertion failure: !sRunningDispatcher || mAccessValid, at /home/amccreight/mc/obj-dbg.noindex/dist/include/mozilla/Dispatcher.h:83

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Assertion failure: !sRunningDispatcher || mAccessValid

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #8)
> Created attachment 8844564 [details]
> Assertion failure: !sRunningDispatcher || mAccessValid

I reproduced this and saved the core file and symbol files if there is interest (the symbols are a bit large to attach to the bug). I hit it a few times today -- the build has some WIP WebRender related patches that are unlikely to have affected this. In this case all I did was started with dom.ipc.processCount set to 1 (although this was not necessary to hit in previous runs), open a tab and go to a new page / wait for it to load, and repeat a few times (images.google.ca, cbc.ca, thestar.com, nationalpost.com), and then I started closing the tabs. During the closing I hit this assert.

Comment 10

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Andrew Osmond [:aosmond] from comment #9)
> (In reply to Andrew McCreight [:mccr8] from comment #8)
> > Created attachment 8844564 [details]
> > Assertion failure: !sRunningDispatcher || mAccessValid
> 
> I reproduced this and saved the core file and symbol files if there is
> interest (the symbols are a bit large to attach to the bug). I hit it a few
> times today -- the build has some WIP WebRender related patches that are
> unlikely to have affected this. In this case all I did was started with
> dom.ipc.processCount set to 1 (although this was not necessary to hit in
> previous runs), open a tab and go to a new page / wait for it to load, and
> repeat a few times (images.google.ca, cbc.ca, thestar.com,
> nationalpost.com), and then I started closing the tabs. During the closing I
> hit this assert.

Could you file a new bug about that? It's a very different assertion.

Comment 11

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Bill McCloskey (:billm) from comment #10)
> (In reply to Andrew Osmond [:aosmond] from comment #9)
> > (In reply to Andrew McCreight [:mccr8] from comment #8)
> > > Created attachment 8844564 [details]
> > > Assertion failure: !sRunningDispatcher || mAccessValid
> > 
> > I reproduced this and saved the core file and symbol files if there is
> > interest (the symbols are a bit large to attach to the bug). I hit it a few
> > times today -- the build has some WIP WebRender related patches that are
> > unlikely to have affected this. In this case all I did was started with
> > dom.ipc.processCount set to 1 (although this was not necessary to hit in
> > previous runs), open a tab and go to a new page / wait for it to load, and
> > repeat a few times (images.google.ca, cbc.ca, thestar.com,
> > nationalpost.com), and then I started closing the tabs. During the closing I
> > hit this assert.
> 
> Could you file a new bug about that? It's a very different assertion.

Will do. I'll try to reproduce on mozilla-central without any other changes.

Comment 12

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 13

[Intermittent Failures Robot]

3 failures in 4710 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 3

Platform breakdown:

• windows7-32: 1
• windows10-64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1342877&startday=2019-07-08&endday=2019-07-14&tree=all

Comment 14

[Chris Peterson [:cpeterson]]

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Comment 15

[Chris Peterson [:cpeterson]]

This bug is not a Fission MVP blocker unless it is breaking some functionality or test with Fission enabled.