Skip to content

S
swiftshader
Commit bf7a8145 by Nicolas Capens
Options

Fix locking a destroyed mutex.

Surface should not lock the resource of a parent texture at destruction, because it can already have been destroyed. For example when a texture's image was bound as a render target, and the texture is deleted by the app, then the image holds the last reference to the texture. When the render target image gets deleted, it first releases its parent texture, and then the underlying surface gets destroyed. This is fixed by synchronizing, by locking and unlocking the (parent) resource, earlier. The derived class is responsible for calling Surface::sync() before releasing the parent resource. Bug chromium:716803 Change-Id: Ifc3685dcf9e25e8419000af65d4bb7407f26bbcb Reviewed-on: https://swiftshader-review.googlesource.com/9750Reviewed-by: 's avatarNicolas Capens <capn@google.com> Tested-by: 's avatarNicolas Capens <capn@google.com>
parent f6a377b0
Showing with 18 additions and 57 deletions
src/Common/Resource.hpp
...@@ -24,7 +24,7 @@ namespace sw ...@@ -24,7 +24,7 @@ namespace sw
PUBLIC, // Application/API access PUBLIC, // Application/API access
PRIVATE, // Renderer access, shared by multiple threads if read-only PRIVATE, // Renderer access, shared by multiple threads if read-only
MANAGED, // Renderer access, shared read/write access if partitioned MANAGED, // Renderer access, shared read/write access if partitioned
DESTRUCT EXCLUSIVE
}; };
class Resource class Resource
......
src/D3D8/Direct3DCubeTexture8.cpp
...@@ -54,8 +54,6 @@ namespace D3D8 ...@@ -54,8 +54,6 @@ namespace D3D8
Direct3DCubeTexture8::~Direct3DCubeTexture8() Direct3DCubeTexture8::~Direct3DCubeTexture8()
{ {
resource->lock(sw::DESTRUCT);
for(unsigned int face = 0; face < 6; face++) for(unsigned int face = 0; face < 6; face++)
{ {
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
...@@ -67,8 +65,6 @@ namespace D3D8 ...@@ -67,8 +65,6 @@ namespace D3D8
} }
} }
} }
resource->unlock();
} }
long Direct3DCubeTexture8::QueryInterface(const IID &iid, void **object) long Direct3DCubeTexture8::QueryInterface(const IID &iid, void **object)
......
src/D3D8/Direct3DTexture8.cpp
...@@ -48,8 +48,6 @@ namespace D3D8 ...@@ -48,8 +48,6 @@ namespace D3D8
Direct3DTexture8::~Direct3DTexture8() Direct3DTexture8::~Direct3DTexture8()
{ {
resource->lock(sw::DESTRUCT);
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
{ {
if(surfaceLevel[level]) if(surfaceLevel[level])
...@@ -58,8 +56,6 @@ namespace D3D8 ...@@ -58,8 +56,6 @@ namespace D3D8
surfaceLevel[level] = 0; surfaceLevel[level] = 0;
} }
} }
resource->unlock();
} }
long Direct3DTexture8::QueryInterface(const IID &iid, void **object) long Direct3DTexture8::QueryInterface(const IID &iid, void **object)
......
src/D3D8/Direct3DVolumeTexture8.cpp
...@@ -49,8 +49,6 @@ namespace D3D8 ...@@ -49,8 +49,6 @@ namespace D3D8
Direct3DVolumeTexture8::~Direct3DVolumeTexture8() Direct3DVolumeTexture8::~Direct3DVolumeTexture8()
{ {
resource->lock(sw::DESTRUCT);
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
{ {
if(volumeLevel[level]) if(volumeLevel[level])
...@@ -59,8 +57,6 @@ namespace D3D8 ...@@ -59,8 +57,6 @@ namespace D3D8
volumeLevel[level] = 0; volumeLevel[level] = 0;
} }
} }
resource->unlock();
} }
long Direct3DVolumeTexture8::QueryInterface(const IID &iid, void **object) long Direct3DVolumeTexture8::QueryInterface(const IID &iid, void **object)
......
src/D3D9/Direct3DCubeTexture9.cpp
...@@ -55,8 +55,6 @@ namespace D3D9 ...@@ -55,8 +55,6 @@ namespace D3D9
Direct3DCubeTexture9::~Direct3DCubeTexture9() Direct3DCubeTexture9::~Direct3DCubeTexture9()
{ {
resource->lock(sw::DESTRUCT);
for(unsigned int face = 0; face < 6; face++) for(unsigned int face = 0; face < 6; face++)
{ {
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
...@@ -68,8 +66,6 @@ namespace D3D9 ...@@ -68,8 +66,6 @@ namespace D3D9
} }
} }
} }
resource->unlock();
} }
long Direct3DCubeTexture9::QueryInterface(const IID &iid, void **object) long Direct3DCubeTexture9::QueryInterface(const IID &iid, void **object)
......
src/D3D9/Direct3DTexture9.cpp
...@@ -49,8 +49,6 @@ namespace D3D9 ...@@ -49,8 +49,6 @@ namespace D3D9
Direct3DTexture9::~Direct3DTexture9() Direct3DTexture9::~Direct3DTexture9()
{ {
resource->lock(sw::DESTRUCT);
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
{ {
if(surfaceLevel[level]) if(surfaceLevel[level])
...@@ -59,8 +57,6 @@ namespace D3D9 ...@@ -59,8 +57,6 @@ namespace D3D9
surfaceLevel[level] = 0; surfaceLevel[level] = 0;
} }
} }
resource->unlock();
} }
long Direct3DTexture9::QueryInterface(const IID &iid, void **object) long Direct3DTexture9::QueryInterface(const IID &iid, void **object)
......
src/D3D9/Direct3DVolumeTexture9.cpp
...@@ -50,8 +50,6 @@ namespace D3D9 ...@@ -50,8 +50,6 @@ namespace D3D9
Direct3DVolumeTexture9::~Direct3DVolumeTexture9() Direct3DVolumeTexture9::~Direct3DVolumeTexture9()
{ {
resource->lock(sw::DESTRUCT);
for(int level = 0; level < sw::MIPMAP_LEVELS; level++) for(int level = 0; level < sw::MIPMAP_LEVELS; level++)
{ {
if(volumeLevel[level]) if(volumeLevel[level])
...@@ -60,8 +58,6 @@ namespace D3D9 ...@@ -60,8 +58,6 @@ namespace D3D9
volumeLevel[level] = 0; volumeLevel[level] = 0;
} }
} }
resource->unlock();
} }
long Direct3DVolumeTexture9::QueryInterface(const IID &iid, void **object) long Direct3DVolumeTexture9::QueryInterface(const IID &iid, void **object)
......
src/OpenGL/common/Image.cpp
...@@ -1182,6 +1182,8 @@ namespace egl ...@@ -1182,6 +1182,8 @@ namespace egl
Image::~Image() Image::~Image()
{ {
sync(); // Wait for any threads that use this image to finish.
if(parentTexture) if(parentTexture)
{ {
parentTexture->release(); parentTexture->release();
......
src/OpenGL/common/Image.hpp
...@@ -254,9 +254,7 @@ private: ...@@ -254,9 +254,7 @@ private:
virtual ~AndroidNativeImage() virtual ~AndroidNativeImage()
{ {
// Wait for any draw calls that use this image to finish sync(); // Wait for any threads that use this image to finish.
resource->lock(sw::DESTRUCT);
resource->unlock();
nativeBuffer->common.decRef(&nativeBuffer->common); nativeBuffer->common.decRef(&nativeBuffer->common);
} }
......
src/OpenGL/libGL/Texture.cpp
...@@ -285,8 +285,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name) ...@@ -285,8 +285,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name)
Texture2D::~Texture2D() Texture2D::~Texture2D()
{ {
resource->lock(sw::DESTRUCT);
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
{ {
if(image[i]) if(image[i])
...@@ -296,8 +294,6 @@ Texture2D::~Texture2D() ...@@ -296,8 +294,6 @@ Texture2D::~Texture2D()
} }
} }
resource->unlock();
mColorbufferProxy = nullptr; mColorbufferProxy = nullptr;
} }
...@@ -642,8 +638,6 @@ TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name) ...@@ -642,8 +638,6 @@ TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name)
TextureCubeMap::~TextureCubeMap() TextureCubeMap::~TextureCubeMap()
{ {
resource->lock(sw::DESTRUCT);
for(int f = 0; f < 6; f++) for(int f = 0; f < 6; f++)
{ {
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
...@@ -656,8 +650,6 @@ TextureCubeMap::~TextureCubeMap() ...@@ -656,8 +650,6 @@ TextureCubeMap::~TextureCubeMap()
} }
} }
resource->unlock();
for(int i = 0; i < 6; i++) for(int i = 0; i < 6; i++)
{ {
mFaceProxies[i] = nullptr; mFaceProxies[i] = nullptr;
......
src/OpenGL/libGLES_CM/Texture.cpp
...@@ -347,8 +347,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name) ...@@ -347,8 +347,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name)
Texture2D::~Texture2D() Texture2D::~Texture2D()
{ {
resource->lock(sw::DESTRUCT);
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
{ {
if(image[i]) if(image[i])
...@@ -358,8 +356,6 @@ Texture2D::~Texture2D() ...@@ -358,8 +356,6 @@ Texture2D::~Texture2D()
} }
} }
resource->unlock();
if(mSurface) if(mSurface)
{ {
mSurface->setBoundTexture(nullptr); mSurface->setBoundTexture(nullptr);
......
src/OpenGL/libGLESv2/Texture.cpp
...@@ -523,8 +523,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name) ...@@ -523,8 +523,6 @@ Texture2D::Texture2D(GLuint name) : Texture(name)
Texture2D::~Texture2D() Texture2D::~Texture2D()
{ {
resource->lock(sw::DESTRUCT);
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
{ {
if(image[i]) if(image[i])
...@@ -534,8 +532,6 @@ Texture2D::~Texture2D() ...@@ -534,8 +532,6 @@ Texture2D::~Texture2D()
} }
} }
resource->unlock();
if(mSurface) if(mSurface)
{ {
mSurface->setBoundTexture(nullptr); mSurface->setBoundTexture(nullptr);
...@@ -1004,8 +1000,6 @@ TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name) ...@@ -1004,8 +1000,6 @@ TextureCubeMap::TextureCubeMap(GLuint name) : Texture(name)
TextureCubeMap::~TextureCubeMap() TextureCubeMap::~TextureCubeMap()
{ {
resource->lock(sw::DESTRUCT);
for(int f = 0; f < 6; f++) for(int f = 0; f < 6; f++)
{ {
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
...@@ -1018,8 +1012,6 @@ TextureCubeMap::~TextureCubeMap() ...@@ -1018,8 +1012,6 @@ TextureCubeMap::~TextureCubeMap()
} }
} }
resource->unlock();
for(int i = 0; i < 6; i++) for(int i = 0; i < 6; i++)
{ {
mFaceProxies[i] = nullptr; mFaceProxies[i] = nullptr;
...@@ -1486,8 +1478,6 @@ Texture3D::Texture3D(GLuint name) : Texture(name) ...@@ -1486,8 +1478,6 @@ Texture3D::Texture3D(GLuint name) : Texture(name)
Texture3D::~Texture3D() Texture3D::~Texture3D()
{ {
resource->lock(sw::DESTRUCT);
for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++) for(int i = 0; i < IMPLEMENTATION_MAX_TEXTURE_LEVELS; i++)
{ {
if(image[i]) if(image[i])
...@@ -1497,8 +1487,6 @@ Texture3D::~Texture3D() ...@@ -1497,8 +1487,6 @@ Texture3D::~Texture3D()
} }
} }
resource->unlock();
if(mSurface) if(mSurface)
{ {
mSurface->setBoundTexture(nullptr); mSurface->setBoundTexture(nullptr);
......
src/Renderer/Surface.cpp
...@@ -1270,9 +1270,11 @@ namespace sw ...@@ -1270,9 +1270,11 @@ namespace sw
Surface::~Surface() Surface::~Surface()
{ {
// Synchronize so we can deallocate the buffers below // sync() must be called before this destructor to ensure all locks have been released.
resource->lock(DESTRUCT); // We can't call it here because the parent resource may already have been destroyed.
resource->unlock(); ASSERT(external.lock == LOCK_UNLOCKED);
ASSERT(internal.lock == LOCK_UNLOCKED);
ASSERT(stencil.lock == LOCK_UNLOCKED);
if(!hasParent) if(!hasParent)
{ {
...@@ -3158,6 +3160,12 @@ namespace sw ...@@ -3158,6 +3160,12 @@ namespace sw
} }
} }
void Surface::sync()
{
resource->lock(EXCLUSIVE);
resource->unlock();
}
bool Surface::isEntire(const SliceRect& rect) const bool Surface::isEntire(const SliceRect& rect) const
{ {
return (rect.x0 == 0 && rect.y0 == 0 && rect.x1 == internal.width && rect.y1 == internal.height && internal.depth == 1); return (rect.x0 == 0 && rect.y0 == 0 && rect.x1 == internal.width && rect.y1 == internal.height && internal.depth == 1);
......
src/Renderer/Surface.hpp
...@@ -291,6 +291,8 @@ namespace sw ...@@ -291,6 +291,8 @@ namespace sw
inline int getStencilPitchB() const; inline int getStencilPitchB() const;
inline int getStencilSliceB() const; inline int getStencilSliceB() const;
void sync(); // Wait for lock(s) to be released
inline int getMultiSampleCount() const; inline int getMultiSampleCount() const;
inline int getSuperSampleCount() const; inline int getSuperSampleCount() const;
...@@ -345,10 +347,9 @@ namespace sw ...@@ -345,10 +347,9 @@ namespace sw
static void setTexturePalette(unsigned int *palette); static void setTexturePalette(unsigned int *palette);
protected: private:
sw::Resource *resource; sw::Resource *resource;
private:
typedef unsigned char byte; typedef unsigned char byte;
typedef unsigned short word; typedef unsigned short word;
typedef unsigned int dword; typedef unsigned int dword;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment