Assertion failure: !isSome(), at /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:795
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
People
(Reporter: jkratzer, Assigned: tnikkel)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, testcase, Whiteboard: [bugmon:confirmed])
Attachments
(3 files)
Testcase found while fuzzing mozilla-central rev cfa4bd8e6f78 (built with --enable-debug). Testcase must be served over HTTP in order to reproduce.
Assertion failure: !isSome(), at /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:795
rax = 0x00007f37ad027629 rdx = 0x0000000000000000
rcx = 0x00005620b3bb6a58 rbx = 0x0000000000000004
rsi = 0x00007f37bf52d8b0 rdi = 0x00007f37bf52c680
rbp = 0x00007ffd188f5f50 rsp = 0x00007ffd188f5f50
r8 = 0x00007f37bf52d8b0 r9 = 0x00007f37c0693780
r10 = 0x0000000000000002 r11 = 0x0000000000000000
r12 = 0x00005620b4ea1b10 r13 = 0x0000000000000001
r14 = 0x00007ffd188f5fe0 r15 = 0x00005620b4ea1b10
rip = 0x00007f37a66c32b3
OS|Linux|0.0.0 Linux 5.3.0-51-generic #44~18.04.2-Ubuntu SMP Thu Apr 23 14:27:18 UTC 2020 x86_64
CPU|amd64|family 6 model 94 stepping 3|8
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|void mozilla::Maybe<nsresult>::emplace<nsresult&>(nsresult&)|hg:hg.mozilla.org/mozilla-central:mfbt/Maybe.h:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|795|0x29
0|1|libxul.so|mozilla::PreloaderBase::NotifyStop(nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/preload/PreloaderBase.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|203|0xc
0|2|libxul.so|imgRequestProxy::OnLoadComplete(bool)|hg:hg.mozilla.org/mozilla-central:image/imgRequestProxy.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1054|0xb
0|3|libxul.so|void mozilla::image::ImageObserverNotifier<mozilla::image::ObserverTable const*>::operator()<void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#7}>(void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#7})|hg:hg.mozilla.org/mozilla-central:image/ProgressTracker.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|351|0x18
0|4|libxul.so|void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)|hg:hg.mozilla.org/mozilla-central:image/ProgressTracker.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|350|0x8
0|5|libxul.so|mozilla::image::ProgressTracker::SyncNotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)|hg:hg.mozilla.org/mozilla-central:image/ProgressTracker.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|368|0x5c
0|6|libxul.so|mozilla::image::RasterImage::NotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::UnorientedPixel> const&, mozilla::Maybe<unsigned int> const&, mozilla::image::DecoderFlags, mozilla::image::SurfaceFlags)|hg:hg.mozilla.org/mozilla-central:image/RasterImage.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1692|0xb
0|7|libxul.so|mozilla::image::RasterImage::NotifyForLoadEvent(unsigned int)|hg:hg.mozilla.org/mozilla-central:image/RasterImage.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|986|0x26
0|8|libxul.so|mozilla::image::RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*, nsresult, bool)|hg:hg.mozilla.org/mozilla-central:image/RasterImage.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|969|0xb
0|9|libxul.so|imgRequest::OnStopRequest(nsIRequest*, nsresult)|hg:hg.mozilla.org/mozilla-central:image/imgRequest.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|788|0x3a
0|10|libxul.so|mozilla::net::HttpChannelChild::DoOnStopRequest(nsIRequest*, nsresult, nsISupports*)|hg:hg.mozilla.org/mozilla-central:netwerk/protocol/http/HttpChannelChild.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1229|0x2e
0|11|libxul.so|mozilla::net::HttpChannelChild::OnStopRequest(nsresult const&, mozilla::net::ResourceTimingStructArgs const&, mozilla::net::nsHttpHeaderArray const&, nsTArray<mozilla::net::ConsoleReportCollected> const&)|hg:hg.mozilla.org/mozilla-central:netwerk/protocol/http/HttpChannelChild.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1094|0x13
0|12|libxul.so|mozilla::net::ChannelEventQueue::FlushQueue()|hg:hg.mozilla.org/mozilla-central:netwerk/ipc/ChannelEventQueue.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|90|0x20
0|13|libxul.so|mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run()|hg:hg.mozilla.org/mozilla-central:netwerk/ipc/ChannelEventQueue.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|148|0x126
0|14|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|146|0x11
0|15|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|1211|0x11
0|16|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|501|0xc
0|17|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|87|0x7
0|18|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|315|0x17
0|19|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|290|0x8
0|20|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|137|0xd
0|21|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|913|0xe
0|22|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|237|0x5
0|23|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|315|0x17
0|24|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|290|0x8
0|25|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|744|0x5
0|26|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|56|0x11
0|27|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|303|0x20
0|28|libc.so.6||||0x21b97
0|29|firefox-bin|<name omitted>|hg:hg.mozilla.org/mozilla-central:mfbt/UniquePtr.h:cfa4bd8e6f789fcca1de2272f0d5b11c0ded913f|253|0x17
|
||
Updated•4 years ago
|
|
||
Comment 1•4 years ago
|
||
We are calling PreloaderBase::NotifyStop more than once. The image code was written by Edgar, asking ni? from him as well.
|
Reporter | |
Comment 2•4 years ago
|
||
Bugmon Analysis:
Unable to reproduce bug using the following builds:
> mozilla-central 20200529155909-60a406d3b53a
> mozilla-central 20200528032513-cfa4bd8e6f78
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.|
|
||
Updated•4 years ago
|
|
||
Comment 3•4 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #1)
We are calling PreloaderBase::NotifyStop more than once.
It is because the imgRequestProxy::OnLoadComplete could possibly be called more than once if we try to validate an image request before the OnStopRequest of the image request is called, detail steps like
- Loads an image that is not in the image cache, so we load it from the network.
- Try to load same image again, hit image cache entry, so we start the validating process.
- imgCacheValidator::OnStartRequest is called and the request is still valid, so we notify the imgRequestProxy in https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/image/imgLoader.cpp#3005, which calls imgRequestProxy::OnLoadComplete.
- Now the imgRequest::OnStopRequest is called, we end up calling imgRequestProxy::OnLoadComplete again, The call stack is as comment 0.
:tnikkel, is this expected behavior? Could you suggest us how to handle this? Thanks!
|
Assignee | |
Comment 4•4 years ago
|
||
(In reply to Edgar Chen [:edgar] from comment #3)
- imgCacheValidator::OnStartRequest is called and the request is still valid, so we notify the imgRequestProxy in https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/image/imgLoader.cpp#3005, which calls imgRequestProxy::OnLoadComplete.
Presumably we get to here
via the UpdateProxies call. So that means that mProgress on the progress tracker for the image has FLAG_LOAD_COMPLETE. How did it get there? It shouldn't be there yet.
|
|
||
Comment 5•4 years ago
|
||
Note that the assertion in my preload code is there to detect misuse of the preload API. If it's fine to simply bypass (not call) NotifyStop the second time, then we can add a flag on PreloaderBase like IsStopped that you can check and not call NotifyStop. It's kinda hack, tho, and I'd be willing to do this only as the last resort.
|
|
||
Comment 6•4 years ago
•
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #4)
Presumably we get to here
via the UpdateProxies call.
Exactly.
So that means that mProgress on the progress tracker for the image has FLAG_LOAD_COMPLETE. How did it get there? It shouldn't be there yet.
Okay, I know what's happened, Image do have FLAG_LOAD_COMPLETE at that time, imgRequest::OnStopRequest has called before, but it doesn't finish notifying observers in https://searchfox.org/mozilla-central/rev/9ad88f80aeedcd3cd7d7f63be07f577861727054/image/ProgressTracker.cpp#350 due to the spin event loop, stack is like https://pastebin.com/2X91amNp. So we see imgRequestProxy::OnLoadComplete from validating one get called first.
So the case is like,
- Loads an image that is not in the image cache, so we load it from the network.
- Try to load same image again, hit image cache entry, so we start the validating process.
- imgRequest::OnStopRequest get called, set mProgress on the progress tracker, but not finish notifying observers due to spin event loop.
- imgCacheValidator::OnStartRequest is called and the request is still valid, so we notify the imgRequestProxy in https://searchfox.org/mozilla-central/rev/559b25eb41c1cbffcb90a34e008b8288312fcd25/image/imgLoader.cpp#3005, which calls imgRequestProxy::OnLoadComplete.
- Leave spin event loop (continue step 3), we end up calling imgRequestProxy::OnLoadComplete again, The call stack is as comment 0.
But it seem the spin event loop just make the imgRequestProxy::OnLoadComplete calls in a different order. Even if there is no spin event loop involved, we would still call imgRequestProxy::OnLoadComplete twice (but in a sane order of course)?
|
|
||
Updated•4 years ago
|
|
|
||
Comment 7•4 years ago
|
||
Why are we validating in-progress loads to begin with? Should we coalesce them?
|
|
Assignee | |
Comment 8•4 years ago
|
||
(In reply to Edgar Chen [:edgar] from comment #6)
But it seem the spin event loop just make the imgRequestProxy::OnLoadComplete calls in a different order. Even if there is no spin event loop involved, we would still call imgRequestProxy::OnLoadComplete twice (but in a sane order of course)?
Yeah, seems right. I guess it's just very rare that we hit such a case, and it doesn't cause (many/bad) problem when it happens (until the preload code added an assert for it).
(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)
Why are we validating in-progress loads to begin with? Should we coalesce them?
Because the current code never considered that case. :) This seems like the easiest way to avoid this situation. We'll see what try thinks of it.
|
|
Assignee | |
Comment 9•4 years ago
|
||
|
||
Updated•4 years ago
|
|
||
Comment 10•4 years ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1c79bd1023d8 Don't kick off a validation network request if the original image network request hasn't finished. r=aosmond
|
|
Assignee | |
Updated•4 years ago
|
|
||
Comment 11•4 years ago
|
||
Backed out changeset 1c79bd1023d8 (bug 1641682) for Build bustage in worker/checkouts/gecko/config/rules.mk
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305906166&repo=autoland&lineNumber=27481
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=1c79bd1023d81bc1c7af1ddf7b15da2ec9a5f351
Backout:
https://hg.mozilla.org/integration/autoland/rev/655eb80ea12e6670aba72c71686c94d68ec3e531
|
|
Assignee | |
Updated•4 years ago
|
|
|
||
Comment 12•4 years ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c0573a579b73 Don't kick off a validation network request if the original image network request hasn't finished. r=aosmond
|
|
||
Comment 13•4 years ago
|
||
Backed out changeset c0573a579b73 (bug 1641682) for WPT failures in /html/canvas/element/imagebitmap/createImageBitmap-invalid-args.html. CLOSED TREE
Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=305945333&repo=autoland&lineNumber=4376
Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=c0573a579b734696d93311cfd789450fb2c08df8
Backout:
https://hg.mozilla.org/integration/autoland/rev/7f9a3a92b1cb8ec921c157faba7ab877d3dbf774
|
|
Assignee | |
Comment 14•4 years ago
|
||
My try server run passed that test. Between try server and review I changed the condition for skipping the validation from "if the original request has load complete or error" to "if the original request has load complete and not error". Which is odd because that makes the change happen in less cases.
|
|
Assignee | |
Comment 15•4 years ago
|
||
stack from comment 6 so it doesn't go away
|
|
Assignee | |
Comment 16•4 years ago
|
||
I filed bug 1645576 for a problem I noticed in our code that we tripped over in this failed test. I changed the patch to not hit this particular problem in this test.
|
|
||
Updated•4 years ago
|
|
|
||
Comment 17•4 years ago
|
||
Pushed by tnikkel@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d925281cab42 Don't kick off a validation network request if the original image network request hasn't finished. r=aosmond
| Comment hidden (Intermittent Failures Robot) |
|
||
Comment 19•4 years ago
|
||
| bugherder | ||
|
||
Updated•4 years ago
|
Description
•