From 88ec3acc834c6c2342de254eef7f904c17e01103 Mon Sep 17 00:00:00 2001 From: Yochai Shefi Simchon Date: Sun, 22 May 2011 16:00:51 +0300 Subject: [PATCH] Fix a mess in contexts/surfaces reference count The main issue was that SmartPtr had an implicit cast to void*, returning its internal pointer. This allowed writing unsafe code, since the internal pointer could be handled without increasing its ref count. So, removed this cast and fixed the various places which relied on it. Also, fix two calls to "destroy" ahich should have been "markForDestruction". The naming is not good, should probably change it in a later patch. Change-Id: Idabc800e97649b2e2404fb7387d25deac70af62e --- .../host/libs/Translator/EGL/EglContext.h | 1 + .../host/libs/Translator/EGL/EglDisplay.cpp | 4 ++-- .../host/libs/Translator/EGL/EglImp.cpp | 21 +++++++++++++------ .../libs/Translator/GLES_CM/GLEScmImp.cpp | 2 +- .../Translator/include/GLcommon/SmartPtr.h | 4 ---- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tools/emulator/opengl/host/libs/Translator/EGL/EglContext.h b/tools/emulator/opengl/host/libs/Translator/EGL/EglContext.h index c162e28e7..5b528ab9f 100644 --- a/tools/emulator/opengl/host/libs/Translator/EGL/EglContext.h +++ b/tools/emulator/opengl/host/libs/Translator/EGL/EglContext.h @@ -53,6 +53,7 @@ public: bool attachImage(unsigned int imageId,ImagePtr img); void detachImage(unsigned int imageId); + ~EglContext(){} private: static unsigned int s_nextContextHndl; EGLNativeContextType m_native; diff --git a/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp b/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp index 893bd1460..1ed7c5936 100644 --- a/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp +++ b/tools/emulator/opengl/host/libs/Translator/EGL/EglDisplay.cpp @@ -108,7 +108,7 @@ bool EglDisplay::removeSurface(SurfacePtr s) { SurfacesHndlMap::iterator it; for(it = m_surfaces.begin(); it!= m_surfaces.end();it++) { - if((*it).second == s.Ptr()) { + if((*it).second.Ptr() == s.Ptr()) { break; } } @@ -135,7 +135,7 @@ bool EglDisplay::removeContext(ContextPtr ctx) { ContextsHndlMap::iterator it; for(it = m_contexts.begin(); it != m_contexts.end();it++) { - if((*it).second == ctx.Ptr()){ + if((*it).second.Ptr() == ctx.Ptr()){ break; } } diff --git a/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp b/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp index 0c3150fed..00b34fb93 100644 --- a/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp +++ b/tools/emulator/opengl/host/libs/Translator/EGL/EglImp.cpp @@ -564,7 +564,7 @@ EGLAPI EGLBoolean EGLAPIENTRY eglDestroySurface(EGLDisplay display, EGLSurface s RETURN_ERROR(EGL_FALSE,EGL_BAD_SURFACE); } - srfc->destroy(); //mark surface for destruction + srfc->markForDestruction(); //mark surface for destruction if(destroySurfaceIfNotCurrent(dpy,srfc)) { //removes surface from the list if not current dpy->removeSurface(surface); } @@ -659,7 +659,7 @@ EGLAPI EGLBoolean EGLAPIENTRY eglDestroyContext(EGLDisplay display, EGLContext c VALIDATE_DISPLAY(display); VALIDATE_CONTEXT(context); - ctx->destroy(); //mark for destruction + ctx->markForDestruction(); //mark for destruction if(destroyContextIfNotCurrent(dpy,ctx)){ //removes the context from the list if it is not current g_eglInfo->getIface(ctx->version())->deleteGLESContext(ctx->getGlesContext()); dpy->removeContext(context); @@ -816,7 +816,7 @@ EGLAPI EGLContext EGLAPIENTRY eglGetCurrentContext(void) { EglDisplay* dpy = static_cast(thread->eglDisplay); EglContext* ctx = static_cast(thread->eglContext); if(dpy && ctx){ - return dpy->getContext(ContextPtr(ctx)); + return dpy->getContext(ctx).Ptr(); } return EGL_NO_CONTEXT; } @@ -830,7 +830,16 @@ EGLAPI EGLSurface EGLAPIENTRY eglGetCurrentSurface(EGLint readdraw) { if(dpy && ctx) { SurfacePtr surface = readdraw == EGL_READ ? ctx->read() : ctx->draw(); - return dpy->getSurface(surface); + if(surface.Ptr()) + { + // This double check is required because a surface might still be + // current after it is destroyed - in which case its handle should + // be invalid, that is EGL_NO_SURFACE should be returned even + // though the surface is current. + surface = dpy->getSurface(surface.Ptr()); + if(surface.Ptr()) + return surface.Ptr(); + } } return EGL_NO_SURFACE; } @@ -858,7 +867,7 @@ EGLAPI EGLBoolean EGLAPIENTRY eglWaitNative(EGLint engine) { SurfacePtr draw = currCtx->draw(); EGLNativeDisplayType nativeDisplay = dpy->nativeType(); - if(read) { + if(read.Ptr()) { if(read->type() == EglSurface::WINDOW && !EglOS::validNativeWin(nativeDisplay,reinterpret_cast(read->native()))) { RETURN_ERROR(EGL_FALSE,EGL_BAD_SURFACE); @@ -868,7 +877,7 @@ EGLAPI EGLBoolean EGLAPIENTRY eglWaitNative(EGLint engine) { RETURN_ERROR(EGL_FALSE,EGL_BAD_SURFACE); } } - if(draw) { + if(draw.Ptr()) { if(draw->type() == EglSurface::WINDOW && !EglOS::validNativeWin(nativeDisplay,reinterpret_cast(draw->native()))) { RETURN_ERROR(EGL_FALSE,EGL_BAD_SURFACE); diff --git a/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp b/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp index a8ff39e20..63c5ce85c 100644 --- a/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp +++ b/tools/emulator/opengl/host/libs/Translator/GLES_CM/GLEScmImp.cpp @@ -1215,7 +1215,7 @@ void glEGLImageTargetTexture2DOES(GLenum target, GLeglImageOES image) // Create the texture object in the underlying EGL implementation, // flag to the OpenGL layer to skip the image creation and map the // current binded texture object to the existing global object. - if (thrd->shareGroup) { + if (thrd->shareGroup.Ptr()) { unsigned int tex = ctx->getBindedTexture(); unsigned int oldGlobal = thrd->shareGroup->getGlobalName(TEXTURE, tex); // Delete old texture object diff --git a/tools/emulator/opengl/host/libs/Translator/include/GLcommon/SmartPtr.h b/tools/emulator/opengl/host/libs/Translator/include/GLcommon/SmartPtr.h index 4bdfbe410..8ac93fb46 100644 --- a/tools/emulator/opengl/host/libs/Translator/include/GLcommon/SmartPtr.h +++ b/tools/emulator/opengl/host/libs/Translator/include/GLcommon/SmartPtr.h @@ -91,10 +91,6 @@ public: return *m_ptr; } - operator void*() const { - return (void *)m_ptr; - } - // This gives STL lists something to compare. bool operator <(const SmartPtr& t1) const { return m_ptr < t1.m_ptr;