eglTerminate() now actually frees up all active egl objects
as specified by the EGL specification, terminated objects's
handles become invalid, the objects themselves are destroyed
when they're not current to some thread.
Change-Id: Id3a4a5736a5bbc3926a9ae8385d43772edb88eeb
diff --git a/opengl/libs/EGL/egl.cpp b/opengl/libs/EGL/egl.cpp
index 1aca4c3..31fe306 100644
--- a/opengl/libs/EGL/egl.cpp
+++ b/opengl/libs/EGL/egl.cpp
@@ -173,21 +173,21 @@
// ----------------------------------------------------------------------------
-egl_display_t* validate_display(EGLDisplay dpy)
-{
+egl_display_t* validate_display(EGLDisplay dpy) {
egl_display_t * const dp = get_display(dpy);
- if (!dp) return setError(EGL_BAD_DISPLAY, (egl_display_t*)NULL);
- if (!dp->isReady()) return setError(EGL_NOT_INITIALIZED, (egl_display_t*)NULL);
+ if (!dp)
+ return setError(EGL_BAD_DISPLAY, (egl_display_t*)NULL);
+ if (!dp->isReady())
+ return setError(EGL_NOT_INITIALIZED, (egl_display_t*)NULL);
return dp;
}
-egl_connection_t* validate_display_config(
- EGLDisplay dpy, EGLConfig config,
- egl_display_t const*& dp)
-{
+egl_connection_t* validate_display_config(EGLDisplay dpy, EGLConfig config,
+ egl_display_t const*& dp) {
dp = validate_display(dpy);
- if (!dp) return (egl_connection_t*) NULL;
+ if (!dp)
+ return (egl_connection_t*) NULL;
if (intptr_t(config) >= dp->numTotalConfigs) {
return setError(EGL_BAD_CONFIG, (egl_connection_t*)NULL);
@@ -199,43 +199,26 @@
return cnx;
}
-EGLBoolean validate_display_context(EGLDisplay dpy, EGLContext ctx)
-{
- egl_display_t const * const dp = validate_display(dpy);
- if (!dp) return EGL_FALSE;
- if (!dp->isAlive())
- return setError(EGL_BAD_DISPLAY, EGL_FALSE);
- if (!get_context(ctx)->isAlive())
- return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- return EGL_TRUE;
-}
-
-EGLBoolean validate_display_surface(EGLDisplay dpy, EGLSurface surface)
-{
- egl_display_t const * const dp = validate_display(dpy);
- if (!dp) return EGL_FALSE;
- if (!dp->isAlive())
- return setError(EGL_BAD_DISPLAY, EGL_FALSE);
- if (!get_surface(surface)->isAlive())
- return setError(EGL_BAD_SURFACE, EGL_FALSE);
- return EGL_TRUE;
-}
-
// ----------------------------------------------------------------------------
EGLImageKHR egl_get_image_for_current_context(EGLImageKHR image)
{
ImageRef _i(image);
- if (!_i.get()) return EGL_NO_IMAGE_KHR;
-
+ if (!_i.get())
+ return EGL_NO_IMAGE_KHR;
+
EGLContext context = egl_tls_t::getContext();
if (context == EGL_NO_CONTEXT || image == EGL_NO_IMAGE_KHR)
return EGL_NO_IMAGE_KHR;
-
+
egl_context_t const * const c = get_context(context);
- if (!c->isAlive())
+ if (c == NULL) // this should never happen
return EGL_NO_IMAGE_KHR;
+ // here we don't validate the context because if it's been marked for
+ // termination, this call should still succeed since it's internal to
+ // EGL.
+
egl_image_t const * const i = get_image(image);
return i->images[c->impl];
}
diff --git a/opengl/libs/EGL/eglApi.cpp b/opengl/libs/EGL/eglApi.cpp
index 210c24d..7d5d010 100644
--- a/opengl/libs/EGL/eglApi.cpp
+++ b/opengl/libs/EGL/eglApi.cpp
@@ -431,10 +431,8 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
-
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
egl_surface_t * const s = get_surface(surface);
EGLBoolean result = s->cnx->egl.eglDestroySurface(
@@ -457,12 +455,10 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(surface);
-
EGLBoolean result(EGL_TRUE);
if (attribute == EGL_CONFIG_ID) {
// We need to remap EGL_CONFIG_IDs
@@ -524,13 +520,13 @@
clearError();
egl_display_t const * const dp = validate_display(dpy);
- if (!dp) return EGL_FALSE;
+ if (!dp)
+ return EGL_FALSE;
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- if (!validate_display_context(dpy, ctx))
- return EGL_FALSE;
egl_context_t * const c = get_context(ctx);
EGLBoolean result = c->cnx->egl.eglDestroyContext(
dp->disp[c->impl].dpy, c->context);
@@ -569,9 +565,9 @@
egl_display_t const * const dp = get_display(dpy);
if (!dp) return setError(EGL_BAD_DISPLAY, EGL_FALSE);
- /* If ctx is not EGL_NO_CONTEXT, read is not EGL_NO_SURFACE, or draw is not
- EGL_NO_SURFACE, then an EGL_NOT_INITIALIZED error is generated if dpy is
- a valid but uninitialized display. */
+ // If ctx is not EGL_NO_CONTEXT, read is not EGL_NO_SURFACE, or draw is not
+ // EGL_NO_SURFACE, then an EGL_NOT_INITIALIZED error is generated if dpy is
+ // a valid but uninitialized display.
if ( (ctx != EGL_NO_CONTEXT) || (read != EGL_NO_SURFACE) ||
(draw != EGL_NO_SURFACE) ) {
if (!dp->isReady()) return setError(EGL_NOT_INITIALIZED, EGL_FALSE);
@@ -583,7 +579,7 @@
SurfaceRef _r(read);
// validate the context (if not EGL_NO_CONTEXT)
- if ((ctx != EGL_NO_CONTEXT) && (!validate_display_context(dpy, ctx))) {
+ if ((ctx != EGL_NO_CONTEXT) && !_c.get()) {
// EGL_NO_CONTEXT is valid
return EGL_FALSE;
}
@@ -682,9 +678,6 @@
ContextRef _c(ctx);
if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- if (!validate_display_context(dpy, ctx))
- return EGL_FALSE;
-
egl_context_t * const c = get_context(ctx);
EGLBoolean result(EGL_TRUE);
@@ -924,10 +917,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(draw);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, draw))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(draw);
return s->cnx->egl.eglSwapBuffers(dp->disp[s->impl].dpy, s->surface);
}
@@ -941,10 +933,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(surface);
return s->cnx->egl.eglCopyBuffers(
dp->disp[s->impl].dpy, s->surface, target);
@@ -984,10 +975,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(surface);
if (s->cnx->egl.eglSurfaceAttrib) {
return s->cnx->egl.eglSurfaceAttrib(
@@ -1005,10 +995,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(surface);
if (s->cnx->egl.eglBindTexImage) {
return s->cnx->egl.eglBindTexImage(
@@ -1026,10 +1015,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(surface);
if (s->cnx->egl.eglReleaseTexImage) {
return s->cnx->egl.eglReleaseTexImage(
@@ -1186,13 +1174,10 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
-
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
egl_surface_t const * const s = get_surface(surface);
-
if (s->cnx->egl.eglLockSurfaceKHR) {
return s->cnx->egl.eglLockSurfaceKHR(
dp->disp[s->impl].dpy, s->surface, attrib_list);
@@ -1208,13 +1193,10 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(surface);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
-
- if (!validate_display_surface(dpy, surface))
- return EGL_FALSE;
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
egl_surface_t const * const s = get_surface(surface);
-
if (s->cnx->egl.eglUnlockSurfaceKHR) {
return s->cnx->egl.eglUnlockSurfaceKHR(
dp->disp[s->impl].dpy, s->surface);
@@ -1232,9 +1214,8 @@
if (ctx != EGL_NO_CONTEXT) {
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_NO_IMAGE_KHR);
- if (!validate_display_context(dpy, ctx))
- return EGL_NO_IMAGE_KHR;
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_NO_IMAGE_KHR);
egl_context_t * const c = get_context(ctx);
// since we have an EGLContext, we know which implementation to use
EGLImageKHR image = c->cnx->egl.eglCreateImageKHR(
@@ -1341,9 +1322,9 @@
EGLContext ctx = eglGetCurrentContext();
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_NO_SYNC_KHR);
- if (!validate_display_context(dpy, ctx))
- return EGL_NO_SYNC_KHR;
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_NO_SYNC_KHR);
+
egl_context_t * const c = get_context(ctx);
EGLSyncKHR result = EGL_NO_SYNC_KHR;
if (c->cnx->egl.eglCreateSyncKHR) {
@@ -1369,9 +1350,8 @@
EGLContext ctx = syncObject->context;
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- if (!validate_display_context(dpy, ctx))
- return EGL_FALSE;
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_FALSE);
EGLBoolean result = EGL_FALSE;
egl_context_t * const c = get_context(ctx);
@@ -1397,12 +1377,10 @@
EGLContext ctx = syncObject->context;
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- if (!validate_display_context(dpy, ctx))
- return EGL_FALSE;
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_FALSE);
egl_context_t * const c = get_context(ctx);
-
if (c->cnx->egl.eglClientWaitSyncKHR) {
return c->cnx->egl.eglClientWaitSyncKHR(
dp->disp[c->impl].dpy, syncObject->sync, flags, timeout);
@@ -1419,17 +1397,16 @@
if (!dp) return EGL_FALSE;
SyncRef _s(sync);
- if (!_s.get()) return setError(EGL_BAD_PARAMETER, EGL_FALSE);
- egl_sync_t* syncObject = get_sync(sync);
+ if (!_s.get())
+ return setError(EGL_BAD_PARAMETER, EGL_FALSE);
+ egl_sync_t* syncObject = get_sync(sync);
EGLContext ctx = syncObject->context;
ContextRef _c(ctx);
- if (!_c.get()) return setError(EGL_BAD_CONTEXT, EGL_FALSE);
- if (!validate_display_context(dpy, ctx))
- return EGL_FALSE;
+ if (!_c.get())
+ return setError(EGL_BAD_CONTEXT, EGL_FALSE);
egl_context_t * const c = get_context(ctx);
-
if (c->cnx->egl.eglGetSyncAttribKHR) {
return c->cnx->egl.eglGetSyncAttribKHR(
dp->disp[c->impl].dpy, syncObject->sync, attribute, value);
@@ -1451,10 +1428,9 @@
if (!dp) return EGL_FALSE;
SurfaceRef _s(draw);
- if (!_s.get()) return setError(EGL_BAD_SURFACE, EGL_FALSE);
+ if (!_s.get())
+ return setError(EGL_BAD_SURFACE, EGL_FALSE);
- if (!validate_display_surface(dpy, draw))
- return EGL_FALSE;
egl_surface_t const * const s = get_surface(draw);
if (s->cnx->egl.eglSetSwapRectangleANDROID) {
return s->cnx->egl.eglSetSwapRectangleANDROID(
diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp
index 272fa44..83aafa6 100644
--- a/opengl/libs/EGL/egl_display.cpp
+++ b/opengl/libs/EGL/egl_display.cpp
@@ -55,6 +55,11 @@
objects.add(object);
}
+void egl_display_t::removeObject(egl_object_t* object) {
+ Mutex::Autolock _l(lock);
+ objects.remove(object);
+}
+
bool egl_display_t::getObject(egl_object_t* object) {
Mutex::Autolock _l(lock);
if (objects.indexOf(object) >= 0) {
@@ -64,15 +69,6 @@
return false;
}
-bool egl_display_t::removeObject(egl_object_t* object) {
- Mutex::Autolock _l(lock);
- if (object->decRef() == 1) {
- objects.remove(object);
- return true;
- }
- return false;
-}
-
EGLDisplay egl_display_t::getFromNativeDisplay(EGLNativeDisplayType disp) {
if (uintptr_t(disp) >= NUM_DISPLAYS)
return NULL;
@@ -255,7 +251,18 @@
}
}
- // TODO: all egl_object_t should be marked for termination
+ // Mark all objects remaining in the list as terminated, unless
+ // there are no reference to them, it which case, we're free to
+ // delete them.
+ size_t count = objects.size();
+ LOGW_IF(count, "eglTerminate() called w/ %d objects remaining", count);
+ for (size_t i=0 ; i<count ; i++) {
+ egl_object_t* o = objects.itemAt(i);
+ o->destroy();
+ }
+
+ // this marks all object handles are "terminated"
+ objects.clear();
refs--;
numTotalConfigs = 0;
diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h
index c194002..8c482c3 100644
--- a/opengl/libs/EGL/egl_display.h
+++ b/opengl/libs/EGL/egl_display.h
@@ -77,12 +77,12 @@
// add object to this display's list
void addObject(egl_object_t* object);
- // remove object from this display's list if it has no reference.
- // returns true if object was removed.
- bool removeObject(egl_object_t* object);
+ // remove object from this display's list
+ void removeObject(egl_object_t* object);
// add reference to this object. returns true if this is a valid object.
bool getObject(egl_object_t* object);
+
static egl_display_t* get(EGLDisplay dpy);
static EGLDisplay getFromNativeDisplay(EGLNativeDisplayType disp);
diff --git a/opengl/libs/EGL/egl_object.cpp b/opengl/libs/EGL/egl_object.cpp
index d4df341..dbf9a01 100644
--- a/opengl/libs/EGL/egl_object.cpp
+++ b/opengl/libs/EGL/egl_object.cpp
@@ -32,16 +32,33 @@
// ----------------------------------------------------------------------------
egl_object_t::egl_object_t(egl_display_t* disp) :
- display(disp), terminated(0), count(1) {
+ display(disp), count(1) {
+ // NOTE: this does an implicit incRef
display->addObject(this);
}
-bool egl_object_t::get() {
- return display->getObject(this);
+egl_object_t::~egl_object_t() {
}
-bool egl_object_t::put() {
- return display->removeObject(this);
+void egl_object_t::terminate() {
+ // this marks the object as "terminated"
+ display->removeObject(this);
+ if (decRef() == 1) {
+ // shouldn't happen because this is called from LocalRef
+ LOGE("egl_object_t::terminate() removed the last reference!");
+ }
+}
+
+void egl_object_t::destroy() {
+ if (decRef() == 1) {
+ delete this;
+ }
+}
+
+bool egl_object_t::get() {
+ // used by LocalRef, this does an incRef() atomically with
+ // checking that the object is valid.
+ return display->getObject(this);
}
// ----------------------------------------------------------------------------
diff --git a/opengl/libs/EGL/egl_object.h b/opengl/libs/EGL/egl_object.h
index ecb2514..3459a8a 100644
--- a/opengl/libs/EGL/egl_object.h
+++ b/opengl/libs/EGL/egl_object.h
@@ -41,64 +41,92 @@
class egl_object_t {
egl_display_t *display;
- volatile int32_t terminated;
mutable volatile int32_t count;
+protected:
+ virtual ~egl_object_t();
+
public:
egl_object_t(egl_display_t* display);
+ void destroy();
- inline bool isAlive() const { return !terminated; }
inline int32_t incRef() { return android_atomic_inc(&count); }
inline int32_t decRef() { return android_atomic_dec(&count); }
private:
+ void terminate();
bool get();
- bool put();
public:
template <typename N, typename T>
- struct LocalRef {
- N* ref;
- LocalRef(T o) : ref(0) {
- N* native = reinterpret_cast<N*>(o);
+ class LocalRef {
+ egl_object_t* ref;
+ LocalRef();
+ LocalRef(const LocalRef* rhs);
+ public:
+ ~LocalRef();
+ explicit LocalRef(egl_object_t* rhs);
+ explicit LocalRef(T o) : ref(0) {
+ egl_object_t* native = reinterpret_cast<N*>(o);
if (o && native->get()) {
ref = native;
}
}
- ~LocalRef() {
- if (ref && ref->put()) {
- delete ref;
- }
- }
inline N* get() {
- return ref;
+ return static_cast<N*>(ref);
}
- void acquire() const {
- if (ref) {
- android_atomic_inc(&ref->count);
- }
- }
- void release() const {
- if (ref) {
- int32_t c = android_atomic_dec(&ref->count);
- // ref->count cannot be 1 prior atomic_dec because we have
- // a reference, and if we have one, it means there was
- // already one before us.
- LOGE_IF(c==1, "refcount is now 0 in release()");
- }
- }
- void terminate() {
- if (ref) {
- ref->terminated = 1;
- release();
- }
- }
+ void acquire() const;
+ void release() const;
+ void terminate();
};
+ template <typename N, typename T>
+ friend class LocalRef;
};
+template<typename N, typename T>
+egl_object_t::LocalRef<N, T>::LocalRef(egl_object_t* rhs) : ref(rhs) {
+ if (ref) {
+ ref->incRef();
+ }
+}
+
+template <typename N, typename T>
+egl_object_t::LocalRef<N,T>::~LocalRef() {
+ if (ref) {
+ ref->destroy();
+ }
+}
+
+template <typename N, typename T>
+void egl_object_t::LocalRef<N,T>::acquire() const {
+ if (ref) {
+ ref->incRef();
+ }
+}
+
+template <typename N, typename T>
+void egl_object_t::LocalRef<N,T>::release() const {
+ if (ref) {
+ if (ref->decRef() == 1) {
+ // shouldn't happen because this is called from LocalRef
+ LOGE("LocalRef::release() removed the last reference!");
+ }
+ }
+}
+
+template <typename N, typename T>
+void egl_object_t::LocalRef<N,T>::terminate() {
+ if (ref) {
+ ref->terminate();
+ }
+}
+
// ----------------------------------------------------------------------------
-struct egl_surface_t: public egl_object_t {
+class egl_surface_t: public egl_object_t {
+protected:
+ ~egl_surface_t() {}
+public:
typedef egl_object_t::LocalRef<egl_surface_t, EGLSurface> Ref;
egl_surface_t(EGLDisplay dpy, EGLConfig config, EGLNativeWindowType win,
@@ -106,8 +134,6 @@
egl_object_t(get_display(dpy)), dpy(dpy), surface(surface),
config(config), win(win), impl(impl), cnx(cnx) {
}
- ~egl_surface_t() {
- }
EGLDisplay dpy;
EGLSurface surface;
EGLConfig config;
@@ -116,7 +142,10 @@
egl_connection_t const* cnx;
};
-struct egl_context_t: public egl_object_t {
+class egl_context_t: public egl_object_t {
+protected:
+ ~egl_context_t() {}
+public:
typedef egl_object_t::LocalRef<egl_context_t, EGLContext> Ref;
egl_context_t(EGLDisplay dpy, EGLContext context, EGLConfig config,
@@ -125,8 +154,6 @@
config(config), read(0), draw(0), impl(impl), cnx(cnx),
version(version) {
}
- ~egl_context_t() {
- }
EGLDisplay dpy;
EGLContext context;
EGLConfig config;
@@ -137,7 +164,10 @@
int version;
};
-struct egl_image_t: public egl_object_t {
+class egl_image_t: public egl_object_t {
+protected:
+ ~egl_image_t() {}
+public:
typedef egl_object_t::LocalRef<egl_image_t, EGLImageKHR> Ref;
egl_image_t(EGLDisplay dpy, EGLContext context) :
@@ -149,7 +179,10 @@
EGLImageKHR images[IMPL_NUM_IMPLEMENTATIONS];
};
-struct egl_sync_t: public egl_object_t {
+class egl_sync_t: public egl_object_t {
+protected:
+ ~egl_sync_t() {}
+public:
typedef egl_object_t::LocalRef<egl_sync_t, EGLSyncKHR> Ref;
egl_sync_t(EGLDisplay dpy, EGLContext context, EGLSyncKHR sync) :