1561667 - Multi-session deadlock conflict when WebGL is running

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Imanol Fernandez]

I can reproduce this in Firefox Reality. if I load a youtube video on one window and any WebGL demo on the other, both webpages get stuck. It seems a conflict between WebGL and video playing together in different active sessions.

Comment 1

[Chris Peterson [:cpeterson]]

We should add multi-window support to GVE to make testing this functionality easier. Randall suspects this is a compositor issue if web content in separate pages is causing them to deadlock.

Is this reproducible with a non-YouTube video? YouTube might be pausing its video when the page loses focus.

Comment 2

[Randall Barker [:rbarker]]

I've got multi GeckoView support working in GVE and can reproduce only when e10s is enabled. I see the same issue in FxR multiscreen with and with out e10s. The error is:

07-05 17:15:00.256 31240 31310 E GLConsumer: [SurfaceTexture-0-31240-20] checkAndUpdateEglState: invalid current EGLContext
07-05 17:15:00.257 31240 31310 W GeckoSurfaceTexture: releaseTexImage() failed
07-05 17:15:00.257 31240 31310 W GeckoSurfaceTexture: java.lang.IllegalStateException: Unable to release texture contents (see logcat for details)
07-05 17:15:00.257 31240 31310 W GeckoSurfaceTexture: 	at android.graphics.SurfaceTexture.nativeReleaseTexImage(Native Method)
07-05 17:15:00.257 31240 31310 W GeckoSurfaceTexture: 	at android.graphics.SurfaceTexture.releaseTexImage(SurfaceTexture.java:252)
07-05 17:15:00.257 31240 31310 W GeckoSurfaceTexture: 	at org.mozilla.gecko.gfx.GeckoSurfaceTexture.releaseTexImage(GeckoSurfaceTexture.java:154)

For GVE it does not seem to matter if the other view is playing a video. One view needs a WebGL page and the other can be about:blank and Gecko with lock up as soon as the second view is made visible with the above error. It might be that a GLContext::MakeCurrent() is missing some where?

Comment 3

[Imanol Fernandez]

I reproduced a similar problem when using a single GV display: https://github.com/MozillaReality/FirefoxReality/issues/1399

The issue happens when we try to resume a GV session running WebGL content (session.setActive(true) and attach it to a display), and we close the previous GV session (I checked that it happens with any of the session.stop() or session.close() calls). If I don't close/stop the previous GV session the new attached GV session runs WebGL correctly

Comment 4

[Imanol Fernandez]

I added some logs in the GeckoSurfaceTexture.java class and confirmed it happens due to a invalid current context when releaseTexImage is called.

2019-07-19 13:43:21.718 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 2 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.734 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 3 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.746 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 4 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.761 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 0 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.774 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 1 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.788 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 2 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.800 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 3 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.814 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 4 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.832 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 0 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.850 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 1 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.858 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 2 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.870 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 3 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.885 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 4 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.900 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 0 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:21.911 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 1 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:22.043 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 2 attached: 3640977408 EGL: -1043286272
2019-07-19 13:43:22.045 7400-7537/org.mozilla.vrbrowser.dev W/GeckoSurfaceTexture: makelele releaseTexImage() failed 2
    java.lang.IllegalStateException: Unable to release texture contents (see logcat for details)
        at android.graphics.SurfaceTexture.nativeReleaseTexImage(Native Method)
        at android.graphics.SurfaceTexture.releaseTexImage(SurfaceTexture.java:253)
        at org.mozilla.gecko.gfx.GeckoSurfaceTexture.releaseTexImage(GeckoSurfaceTexture.java:158)
2019-07-19 13:43:22.095 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 3 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:22.341 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 4 attached: 3640977408 EGL: -1044160192
2019-07-19 13:43:22.562 7400-7537/org.mozilla.vrbrowser.dev E/VRB: releaseTexImage 0 attached: 3640977408 EGL: -1043286272

The bug seems to be in TextureHostOGL.cpp which calls releaseTexImage before ensuring the SurfaceTexture is attached to the correct GL context. I tried to add a EnsureAttached() call and both problems were fixed! I'll post a patch

Comment 5

[Imanol Fernandez]

Attachment: Bug 1561667 - Ensure that SurfaceTexture is attached before calling ReleaseTexImage.

Ensure that SurfaceTexture is attached before calling ReleaseTexImage.

Comment 6

[Chris Peterson [:cpeterson]]

We should uplift this fix to GV 69 Beta if the session deadlock can affect Fenix users with multiple tabs.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdd04fa751fc
Ensure that SurfaceTexture is attached before calling ReleaseTexImage. r=jgilbert,rbarker

Comment 8

[Noemi Erli[:noemi_erli]]

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

Comment 9

[Chris Peterson [:cpeterson]]

Imanol, can you please request uplift of your fix to GeckoView 69 Beta?

This session deadlock may affect Fenix users with multiple tabs.

Comment 10

[Imanol Fernandez]

Comment on attachment 9079378 [details]
Bug 1561667 - Ensure that SurfaceTexture is attached before calling ReleaseTexImage.

Beta/Release Uplift Approval Request

• User impact if declined: Session deadlock that may affect Fenix users with multiple tabs.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a single line commit and It actually fixes a freeze.
• String changes made/needed:

Comment 11

[Imanol Fernandez]

@cpeterson my correct email is [e-mail]

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9079378 [details]
Bug 1561667 - Ensure that SurfaceTexture is attached before calling ReleaseTexImage.

Simple fix for a Fenix freeze. Approved for GV69.

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4be810d4e5b7