From fb87e54a9af8bc5063ca4deebe81d90126992480 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 30 Jan 2012 18:20:52 -0800 Subject: [PATCH] fix a race condition in eglMakeCurrent() it would happen when a context was made non-current, in this case we would call the implementation's eglMakeCurrent() which would succeed, if we're rescheduled at that point, another eglMakeCurrent() could make that context current to another thread, however, when we came back to the original call we would overwrite egl_context_t internal state. this is fixed by moving the critical section under egl_display_t's lock. Change-Id: I743c85696e13263d3a9570824940263df0caacdc --- opengl/libs/EGL/eglApi.cpp | 39 ++++--------------------------- opengl/libs/EGL/egl_display.cpp | 41 +++++++++++++++++++++++++++++++++ opengl/libs/EGL/egl_display.h | 7 +++++- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/opengl/libs/EGL/eglApi.cpp b/opengl/libs/EGL/eglApi.cpp index 9379c53c3..73aab26cc 100644 --- a/opengl/libs/EGL/eglApi.cpp +++ b/opengl/libs/EGL/eglApi.cpp @@ -566,26 +566,6 @@ EGLBoolean eglDestroyContext(EGLDisplay dpy, EGLContext ctx) return result; } -static void loseCurrent(egl_context_t * cur_c) -{ - if (cur_c) { - egl_surface_t * cur_r = get_surface(cur_c->read); - egl_surface_t * cur_d = get_surface(cur_c->draw); - - // by construction, these are either 0 or valid (possibly terminated) - // it should be impossible for these to be invalid - ContextRef _cur_c(cur_c); - SurfaceRef _cur_r(cur_r); - SurfaceRef _cur_d(cur_d); - - cur_c->onLooseCurrent(); - - _cur_c.release(); - _cur_r.release(); - _cur_d.release(); - } -} - EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx) { @@ -662,21 +642,13 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, impl_read = r->surface; } - EGLBoolean result; - if (c) { - result = c->cnx->egl.eglMakeCurrent( - dp->disp[c->impl].dpy, impl_draw, impl_read, impl_ctx); - } else { - result = cur_c->cnx->egl.eglMakeCurrent( - dp->disp[cur_c->impl].dpy, impl_draw, impl_read, impl_ctx); - } + EGLBoolean result = const_cast(dp)->makeCurrent(c, cur_c, + draw, read, ctx, + impl_draw, impl_read, impl_ctx); if (result == EGL_TRUE) { - - loseCurrent(cur_c); - - if (ctx != EGL_NO_CONTEXT) { + if (c) { setGLHooksThreadSpecific(c->cnx->hooks[c->version]); egl_tls_t::setContext(ctx); #if EGL_TRACE @@ -686,7 +658,6 @@ EGLBoolean eglMakeCurrent( EGLDisplay dpy, EGLSurface draw, _c.acquire(); _r.acquire(); _d.acquire(); - c->onMakeCurrent(draw, read); } else { setGLHooksThreadSpecific(&gHooksNoContext); egl_tls_t::setContext(EGL_NO_CONTEXT); @@ -1179,7 +1150,7 @@ EGLBoolean eglReleaseThread(void) clearError(); // If there is context bound to the thread, release it - loseCurrent(get_context(getContext())); + egl_display_t::loseCurrent(get_context(getContext())); for (int i=0 ; iread); + egl_surface_t * cur_d = get_surface(cur_c->draw); + + // by construction, these are either 0 or valid (possibly terminated) + // it should be impossible for these to be invalid + ContextRef _cur_c(cur_c); + SurfaceRef _cur_r(cur_r); + SurfaceRef _cur_d(cur_d); + + cur_c->onLooseCurrent(); + + _cur_c.release(); + _cur_r.release(); + _cur_d.release(); + } +} + +EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c, + EGLSurface draw, EGLSurface read, EGLContext ctx, + EGLSurface impl_draw, EGLSurface impl_read, EGLContext impl_ctx) +{ + Mutex::Autolock _l(lock); + EGLBoolean result; + if (c) { + result = c->cnx->egl.eglMakeCurrent( + disp[c->impl].dpy, impl_draw, impl_read, impl_ctx); + } else { + result = cur_c->cnx->egl.eglMakeCurrent( + disp[cur_c->impl].dpy, impl_draw, impl_read, impl_ctx); + } + if (result == EGL_TRUE) { + loseCurrent(cur_c); + if (c) { + c->onMakeCurrent(draw, read); + } + } + return result; +} // ---------------------------------------------------------------------------- }; // namespace android diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h index 042ae07a9..4479e00ed 100644 --- a/opengl/libs/EGL/egl_display.h +++ b/opengl/libs/EGL/egl_display.h @@ -39,6 +39,7 @@ namespace android { // ---------------------------------------------------------------------------- class egl_object_t; +class egl_context_t; class egl_connection_t; // ---------------------------------------------------------------------------- @@ -84,10 +85,14 @@ public: // add reference to this object. returns true if this is a valid object. bool getObject(egl_object_t* object) const; - static egl_display_t* get(EGLDisplay dpy); static EGLDisplay getFromNativeDisplay(EGLNativeDisplayType disp); + EGLBoolean makeCurrent(egl_context_t* c, egl_context_t* cur_c, + EGLSurface draw, EGLSurface read, EGLContext ctx, + EGLSurface impl_draw, EGLSurface impl_read, EGLContext impl_ctx); + static void loseCurrent(egl_context_t * cur_c); + inline bool isReady() const { return (refs > 0); } inline bool isValid() const { return magic == '_dpy'; } inline bool isAlive() const { return isValid(); }