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

We are calling PreloaderBase::NotifyStop more than once. The image code was written by Edgar, asking ni? from him as well.

(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

1. Loads an image that is not in the image cache, so we load it from the network.
2. Try to load same image again, hit image cache entry, so we start the validating process.
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.
4. 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!

(In reply to Edgar Chen [:edgar] from comment #3)

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

https://searchfox.org/mozilla-central/rev/7cadba1d8b8feaec4615f5bb98aac4b8a719793c/image/ProgressTracker.cpp#351

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.

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.

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

Presumably we get to here

https://searchfox.org/mozilla-central/rev/7cadba1d8b8feaec4615f5bb98aac4b8a719793c/image/ProgressTracker.cpp#351

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,

1. Loads an image that is not in the image cache, so we load it from the network.
2. Try to load same image again, hit image cache entry, so we start the validating process.
3. imgRequest::OnStopRequest get called, set mProgress on the progress tracker, but not finish notifying observers due to spin event loop.
4. 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.
5. 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)?

Why are we validating in-progress loads to begin with? Should we coalesce them?

(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.

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

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

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.

stack from comment 6 so it doesn't go away

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.

https://hg.mozilla.org/mozilla-central/rev/d925281cab42