1290831 - Hit MOZ_CRASH(Unexpected error with MOZ_GL_DEBUG_ABORT_ON_ERROR. (Run with MOZ_GL_DEBUG_ABORT_ON_ERROR=0 to disable)) at GLContext.h:759

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: complete stack

Found via bughunter and seems affected beta -> nightly 

Reproduced with latest m-c debug tinderbox build on windows 

Steps to reproduce:
-> http://www.anj.fyi/behold/
-> Load Hit MOZ_CRASH(Unexpected error with MOZ_GL_DEBUG_ABORT_ON_ERROR. (Run with MOZ_GL_DEBUG_ABORT_ON_ERROR=0 to disable)) at c:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\src\gfx\gl\GLContext.h:759

Comment 1

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:
bughunter

Comment 2

[Kelsey Gilbert [:jgilbert]]

about:support please.

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I can't reproduce with the build:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-win64-debug/1470060589/
at win10.

Comment 4

[Carsten Book [:Tomcat]]

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #3)
> I can't reproduce with the build:
> http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-
> win64-debug/1470060589/
> at win10.

it seems you might reload the page several times via shift+reload this works for me to trigger the crash on win 7

Comment 5

[Carsten Book [:Tomcat]]

about:support 
(its a win7 vm on a fusion 8 mac 10.11 mbp).

Graphics
--------

Features
Compositing: Basic
Asynchronous Pan/Zoom: wheel input enabled; touch input enabled
WebGL Renderer: Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_5_0 ps_5_0)
WebGL2 Renderer: WebGL creation failed: * Refused to create native OpenGL context because of blacklist entry: FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR * Exhausted GL driver options.
Hardware H264 Decoding: No; Hardware video decoding disabled or blacklisted
Audio Backend: wasapi
Direct2D: Blocked for your graphics card because of unresolved driver issues.
DirectWrite: false (6.2.9200.17568)
GPU #1
Active: Yes
Description: VMware SVGA 3D
Vendor ID: 0x15ad
Device ID: 0x0405
Driver Version: 8.15.1.33
Driver Date: 10-16-2015
Drivers: vm3dum vm3dum_10
Subsys ID: 040515ad
RAM: 384

Diagnostics
AzureCanvasAccelerated: 0
AzureCanvasBackend: skia
AzureContentBackend: cairo
AzureFallbackCanvasBackend: cairo
Decision Log
D3D11_COMPOSITING:
Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR
D3D9_COMPOSITING:
Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR
DIRECT2D:
unavailable by default: Direct2D requires Direct3D 11 compositing
D3D11_HW_ANGLE:
unavailable by default: D3D11 compositing is disabled
disabled by env: D3D11 compositing is disabled

---------------------

Comment 6

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Jerry or Jeff, any luck reproducing this?

Comment 7

[Kelsey Gilbert [:jgilbert]]

I have no bandwidth for this.

Comment 8

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

David or Milan, can you help? Not sure how serious this is.  Hitting MOZ_CRASH is usually not good. If I don't need to worry about it for 49, let's defer it to 51.

Comment 9

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I still can't reproduce this at win7.
I will try a win7 vm on fusion 8 later.

Comment 10

[Bob Clary [:bc] (inactive)]

Attachment: aurora debug stack

I ran this url in bughunter on windows 7 again and found only one aurora debug crash @ gl::GLContext::AfterGLCall(char const *) [GLContext.h:264adddee81b : 758 + 0x22]

It may be intermittent. If you need, I can try to reproduce more aggressively.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

First comment - we weren't allowing this configuration (software ANGLE for WebGL, basic compositor) until bug 1271770, which landed on nightly in 50, then got uplifted to 49 on July 27th.  I would imagine that to be the cause - exposing an existing bug because of the additional number of people that can now run WebGL, rather than a new bug.

Based on the debug stack - perhaps VMWare doesn't do mipmaps properly.

Comment 12

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Milan, that sounds like a potentially good thing (more people able to run WebGL). Deferring this to 50 at this point as we are heading into 49 beta 8 now.

Comment 13

[Peter Chang[:pchang]]

Michael, please help to reproduce this crash in win 7 VM. I think it might go different flow after bug 1271770 or bug 1297965.

Comment 14

[Michael Leu[:Lenzak](UTC+8)]

OK, I will prepare a Windows 7 VM running under VMWare Fusion

Comment 15

[Michael Leu[:Lenzak](UTC+8)]

Hi Tomcat, 

I have set up a windows 7 VM with same VMWare SVGA driver version as the about:support you posted.

But I cannot reproduce the crash, can you pull the latest mozilla-central code and test it again?

In addition, what is the build number of your VMWare fusion?

I think maybe this issue is related to the GFX emulation inside VMWare Fusion.

Comment 16

[Carsten Book [:Tomcat]]

Attachment: latest m-c stack

crashed after a long time 

mozilla-central tinderbox build (debug, windows 7) based on https://hg.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38

Comment 17

[Carsten Book [:Tomcat]]

(In reply to Michael Leu[:lenzak800](UTC+8)[PTO 10/6 ~ 10/13] from comment #15)
> Hi Tomcat, 
> 
> I have set up a windows 7 VM with same VMWare SVGA driver version as the
> about:support you posted.
> 
> But I cannot reproduce the crash, can you pull the latest mozilla-central
> code and test it again?
> 
> In addition, what is the build number of your VMWare fusion?
> 
> I think maybe this issue is related to the GFX emulation inside VMWare
> Fusion.

Hi Michael, its Version 8.5.0 (4352717) Fusion Mac - also attached a m-c stack from today

Comment 18

[Michael Leu[:Lenzak](UTC+8)]

OK, I will open it and wait for several minutes.
BTW, here is my about:support.

Graphics
Features
Compositing	Basic
Asynchronous Pan/Zoom	wheel input enabled; touch input enabled
WebGL Renderer	Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_4_1 ps_4_1)
WebGL2 Renderer	Google Inc. -- ANGLE (Software Adapter Direct3D11 vs_4_1 ps_4_1)
Hardware H264 Decoding	No; Hardware video decoding disabled or blacklisted
Audio Backend	unknown
Direct2D	Blocked for your graphics card because of unresolved driver issues.
DirectWrite	false (6.1.7601.17514)
GPU #1
Active	Yes
Description	VMware SVGA 3D
Vendor ID	0x15ad
Device ID	0x0405
Driver Version	8.15.1.33
Driver Date	10-16-2015
Drivers	vm3dum vm3dum_10
Subsys ID	040515ad
RAM	8
Diagnostics
AzureCanvasAccelerated	0
AzureCanvasBackend	skia
AzureContentBackend	cairo
AzureFallbackCanvasBackend	cairo
failures	[GFX1-]: Refresh driver waiting for the compositor for1.04809 seconds.
Decision Log
D3D11_COMPOSITING	
Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR
D3D9_COMPOSITING	
Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR
DIRECT2D	
unavailable by default: Direct2D requires Direct3D 11 compositing
D3D11_HW_ANGLE	
unavailable by default: D3D11 compositing is disabled
disabled by env: D3D11 compositing is disabled

Comment 19

[Michael Leu[:Lenzak](UTC+8)]

OK, I can reproduce it after I move the VM to same VMWare Fusion build.

It seems that only windows 7 32bit has this issue.

Comment 20

[Gerry Chang [:gchang]]

Track 51+ as it can be reproduced.

Comment 21

[Michael Leu[:Lenzak](UTC+8)]

I found that this crash always happens after a buffer allocation failure.
This website has a texture with dimension 8192*8192, which requires 256MB of memory.

And this allocation which utilizes calloc sometime fails with a nullptr return here.
https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=Unable+to+allocate+buffer+during+conversion&redirect_type=single#253

But both the VM's system or GFX memory are sufficient to allocate, so I am still trying to figure out why this memory allocation failure happen and why this happens intermittently.

Comment 22

[Michael Leu[:Lenzak](UTC+8)]

OK, I think I found the reason.

calloc returns nullptr when no consecutive required memory space available.

Since 256MB is a very big value, it will be fail-prone.

It also explains why only 32bit windows happens because 64bit has a much larger address space.

So it may be a problem of our error handling, we should have bailed out from the texture operations before it hit MOZ_CRASH.

Comment 23

[Peter Chang[:pchang]]

(In reply to Michael Leu[:lenzak800](UTC+8)[PTO 10/6 ~ 10/13] from comment #22)
> OK, I think I found the reason.
> 
> calloc returns nullptr when no consecutive required memory space available.
> 
> Since 256MB is a very big value, it will be fail-prone.
> 
> It also explains why only 32bit windows happens because 64bit has a much
> larger address space.
> 
> So it may be a problem of our error handling, we should have bailed out from
> the texture operations before it hit MOZ_CRASH.

Good catch, Michael.

Comment 24

[Michael Leu[:Lenzak](UTC+8)]

So here is how the crash happens.

1. The website calls TexImage2D, requesting a large buffer.

2. Under certain environment, we use CPU-side conversion in ConvertIfNeeded
https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=ConvertIfNeeded&redirect_type=direct#171

3. We cannot allocate such a large buffer in CPU side, so the calloc returns null, following an OOM exception thrown.
https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=ConvertIfNeeded&redirect_type=direct#253

4. The website still calls GenerateMipmap with this faulty texture, so we got an INVALID_OPERATION exception, then we crashed because it is a debug build with MOZ_GL_DEBUG_ABORT_ON_ERROR set.

This whole flow seems to be normal, we have told the website we're out of memory but they still use it, so we pop out more error, and it crashes because its debug configuration.

Jeff, do you have any suggestion about this one?

Maybe it is just a normal reaction instead of a bug.

Comment 25

[Kelsey Gilbert [:jgilbert]]

Great run-down.

#4 should not happen. We should be validating that it's valid to send glGenerateMipmaps to the driver before we call it. The fix is the repair our GenerateMipmap validation.

Comment 26

[Michael Leu[:Lenzak](UTC+8)]

Attachment: Prevent BaseImageInfo being initialized when TexOrSubImage fails.

Comment 27

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8804598 [details] [diff] [review]
Prevent BaseImageInfo being initialized when TexOrSubImage fails.

Review of attachment 8804598 [details] [diff] [review]:
-----------------------------------------------------------------

I found that we do have some checks when we call GenerateMipmap,
the problem is that if blob->TexOrSubImage fails with a false return before it reaches the real GL part (DoTexSubImage),
it wouldn't be treated as failure because glError is 0 since we didn't reach real GL call.
https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLTextureUpload.cpp#1423

So we continue to execute, initializing the ImageInfo inside WebGLTexture which make other further calls thought it is a valid texture.

This patch add checks for blob->TexOrSubImage, bailing out if it returns false, which prevent the ImageInfo from falsely initialized.

So further texture calls will bail out by their own checking.

Comment 28

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8804598 [details] [diff] [review]
Prevent BaseImageInfo being initialized when TexOrSubImage fails.

Review of attachment 8804598 [details] [diff] [review]:
-----------------------------------------------------------------

Feels like we should do similar error reporting as we do for out of memory scenarios, rather than just return?

Comment 29

[Michael Leu[:Lenzak](UTC+8)]

(In reply to Milan Sreckovic [:milan] from comment #28)
> Comment on attachment 8804598 [details] [diff] [review]
> Prevent BaseImageInfo being initialized when TexOrSubImage fails.
> 
> Review of attachment 8804598 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Feels like we should do similar error reporting as we do for out of memory
> scenarios, rather than just return?

It has thrown exception here so the website should have known they run out of memory.
https://dxr.mozilla.org/mozilla-central/source/dom/canvas/TexUnpackBlob.cpp?q=ConvertIfNeeded&redirect_type=direct#253

The problem is that it continue to initialize ImageInfo after the exception thrown,
so further web gl calls don't know the texture is faulty.

Since it has thrown OM exception, I think we can just return.

Comment 30

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Too late for 50, we should plan to fix this in 51.

Comment 31

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1290831 - Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. -

Review commit: https://reviewboard.mozilla.org/r/93320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93320/

Comment 32

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8804598 [details] [diff] [review]
Prevent BaseImageInfo being initialized when TexOrSubImage fails.

Review of attachment 8804598 [details] [diff] [review]:
-----------------------------------------------------------------

This is the right idea, but I didn't really make the guarantee tight when I wrote this before.
I uploaded patch that expands on yours.

::: dom/canvas/WebGLTextureUpload.cpp
@@ +1336,5 @@
>  
>      GLenum glError;
> +    if (!blob->TexOrSubImage(isSubImage, needsRespec, funcName,
> +                             this, target, level, driverUnpackInfo,
> +                             xOffset, yOffset, zOffset, &glError)) {

{ goes on its own line for multi-line conditionals.

Comment 33

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8811103 [details]
Bug 1290831 - Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. -

https://reviewboard.mozilla.org/r/93320/#review93480

Thanks for your suggestion. :)

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/971de933cd5d
Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. - r=cleu

Comment 35

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/971de933cd5d

Comment 36

[Gerry Chang [:gchang]]

Hi Michael,
could you please nominate this uplift to Beta51 and Aurora52 if this patch is not too risky?

Comment 37

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8811103 [details]
Bug 1290831 - Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. -

Approval Request Comment
[Feature/regressing bug #]: Bug1290831
[User impact if declined]: Users will encounter unstable webGL when we perform CPu-side texture processing.
[Describe test coverage new/current, TreeHerder]: Hand test and see no crash in debug build.
[Risks and why]: Low, it just slightly modifies the operation validation process.
[String/UUID change made/needed]: N/A

Comment 38

[Gerry Chang [:gchang]]

Hi Michael, 
Because WebGL 2 will be shipped in 51, isn't this worth uplifting to Beta51?

Comment 39

[Kelsey Gilbert [:jgilbert]]

Yes.

Comment 40

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8811103 [details]
Bug 1290831 - Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. -

Approval Request Comment
[Feature/regressing bug #]: webgl2
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 41

[Gerry Chang [:gchang]]

Comment on attachment 8811103 [details]
Bug 1290831 - Clarify TexUnpackBlob::TexOrSubImage's fallibility and update callers. -

Fix an issue related to WebGL 2. Beta51+ and Aurora52+. Should be in 51 beta 2.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/867d03558dc2

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2bf05af8c02d