New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 603223 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

eglInitialize/eglTerminate create and destroy the current context TLS Key causing problems when multiple egl::Display objects exist

Project Member Reported by bsalo...@google.com, Apr 13 2016

Issue description

The context for this bug is using the command buffer shared library in Skia tests. Skia loads the library and then interacts with it using its exported EGL and GL interface.

Currently every call to eglGetDisplay() returns a new egl::Display object. When eglInitialize() is called Display::Initialize() is called on the display which in turn calls gles2::Initialize(). This last function sets up the TLS key that is used to store the current GL context, called g_gl_context_key.

Likewise, when eglTerminate is called on a display egl::Display::~Display() is called which calls gles2::Terminate(). gles2::Terminate frees g_gl_context_key.

The Skia test suite sometimes creates multiple GL contexts. The Skia GL context wrapper class calls eglGetDisplay(GL_DEFAULT_DISPLAY) and eglInitiailize() when a context is created and eglTerminate() when the context is destroyed. Each context gets its own display since eglGetDisplay always returns a new object.

A problem arises if we create two contexts, A and B. If A is destroyed it becomes impossible to safely use B because the TLS key for the current context no longer exists.

I can probably work around this, but am filing the bug to document my findings.
 

Comment 1 by kbr@chromium.org, Apr 13 2016

Cc: sunn...@chromium.org
Components: Internals>GPU>Internals

Comment 2 by kbr@chromium.org, Apr 13 2016

Not sure who is maintaining the command buffer as a shared library right now.

Comment 3 by bsalo...@google.com, Apr 13 2016

This actually is not so easy to work around. I thought we could just use a global EGLDisplay for all contexts. However, the implementation only allows for one GL context per egl::Display. I suppose I just skip the eglTerminate() call for now.

Comment 4 by bsalo...@google.com, Apr 13 2016

Leaking egl::Displays eventually causes a crash. However, deferring eglTerminate() calls until no displays are in use by Skia works as we only have a small number of contexts in use at any given time.
It will be fixed in the work for bug 581634
 https://codereview.chromium.org/1714883002/
Project Member

Comment 6 by bugdroid1@chromium.org, May 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ccf452e99ca69ae893c2eabe5f3afcd321abf2f

commit 5ccf452e99ca69ae893c2eabe5f3afcd321abf2f
Author: skia-deps-roller <skia-deps-roller@chromium.org>
Date: Tue May 17 14:29:20 2016

Roll src/third_party/skia/ 4933d0baa..aac0cb07c (1 commit).

https://chromium.googlesource.com/skia.git/+log/4933d0baadae..aac0cb07ca44

$ git log 4933d0baa..aac0cb07c --date=short --no-merges --format='%ad %ae %s'
2016-05-17 kkinnunen Remove workarounds initializing command buffer EGL

BUG=581634, 603223 

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=reed@google.com

Review-Url: https://codereview.chromium.org/1980393002
Cr-Commit-Position: refs/heads/master@{#394121}

[modify] https://crrev.com/5ccf452e99ca69ae893c2eabe5f3afcd321abf2f/DEPS

Project Member

Comment 7 by sheriffbot@chromium.org, Jun 3 2016

Labels: Hotlist-Google
Status: Fixed (was: Available)
should be fixed.

Sign in to add a comment