Fix use-after-free when deleting share contexts.
The pattern of gen context, share context, free context, then allocate
a shared GL object in the second context would cause a use-after-free
of the ContextImpl as a GLFactory. Fix this by passing the factory
as a parameter to the resource manager allocation methods instead of
storing the factory pointer. This allows the same ResourceManager to
work with separate Context implementations, which will work with
non-virtual contexts.
BUG=612931
Change-Id: Ifceeb893bebd072f318963d935ff9d17181f5305
Reviewed-on: https://chromium-review.googlesource.com/347463
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp
index 2ef7972..750f4a4 100644
--- a/src/libANGLE/Context.cpp
+++ b/src/libANGLE/Context.cpp
@@ -162,7 +162,7 @@
}
else
{
- mResourceManager = new ResourceManager(mImplementation.get());
+ mResourceManager = new ResourceManager();
}
mData.resourceManager = mResourceManager;
@@ -411,12 +411,13 @@
GLuint Context::createProgram()
{
- return mResourceManager->createProgram();
+ return mResourceManager->createProgram(mImplementation.get());
}
GLuint Context::createShader(GLenum type)
{
- return mResourceManager->createShader(mImplementation->getNativeLimitations(), type);
+ return mResourceManager->createShader(mImplementation.get(),
+ mImplementation->getNativeLimitations(), type);
}
GLuint Context::createTexture()
@@ -431,7 +432,7 @@
GLsync Context::createFenceSync()
{
- GLuint handle = mResourceManager->createFenceSync();
+ GLuint handle = mResourceManager->createFenceSync(mImplementation.get());
return reinterpret_cast<GLsync>(static_cast<uintptr_t>(handle));
}
@@ -706,13 +707,13 @@
void Context::bindArrayBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setArrayBufferBinding(buffer);
}
void Context::bindElementArrayBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getVertexArray()->setElementArrayBuffer(buffer);
}
@@ -726,7 +727,7 @@
}
else
{
- texture = mResourceManager->checkTextureAllocation(handle, target);
+ texture = mResourceManager->checkTextureAllocation(mImplementation.get(), handle, target);
}
ASSERT(texture);
@@ -747,7 +748,8 @@
void Context::bindRenderbuffer(GLuint renderbufferHandle)
{
- Renderbuffer *renderbuffer = mResourceManager->checkRenderbufferAllocation(renderbufferHandle);
+ Renderbuffer *renderbuffer =
+ mResourceManager->checkRenderbufferAllocation(mImplementation.get(), renderbufferHandle);
mState.setRenderbufferBinding(renderbuffer);
}
@@ -760,13 +762,14 @@
void Context::bindSampler(GLuint textureUnit, GLuint samplerHandle)
{
ASSERT(textureUnit < mCaps.maxCombinedTextureImageUnits);
- Sampler *sampler = mResourceManager->checkSamplerAllocation(samplerHandle);
+ Sampler *sampler =
+ mResourceManager->checkSamplerAllocation(mImplementation.get(), samplerHandle);
mState.setSamplerBinding(textureUnit, sampler);
}
void Context::bindGenericUniformBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setGenericUniformBufferBinding(buffer);
}
@@ -775,13 +778,13 @@
GLintptr offset,
GLsizeiptr size)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setIndexedUniformBufferBinding(index, buffer, offset, size);
}
void Context::bindGenericTransformFeedbackBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getCurrentTransformFeedback()->bindGenericBuffer(buffer);
}
@@ -790,31 +793,31 @@
GLintptr offset,
GLsizeiptr size)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.getCurrentTransformFeedback()->bindIndexedBuffer(index, buffer, offset, size);
}
void Context::bindCopyReadBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setCopyReadBufferBinding(buffer);
}
void Context::bindCopyWriteBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setCopyWriteBufferBinding(buffer);
}
void Context::bindPixelPackBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setPixelPackBufferBinding(buffer);
}
void Context::bindPixelUnpackBuffer(GLuint bufferHandle)
{
- Buffer *buffer = mResourceManager->checkBufferAllocation(bufferHandle);
+ Buffer *buffer = mResourceManager->checkBufferAllocation(mImplementation.get(), bufferHandle);
mState.setPixelUnpackBufferBinding(buffer);
}
@@ -1910,7 +1913,7 @@
void Context::samplerParameteri(GLuint sampler, GLenum pname, GLint param)
{
- mResourceManager->checkSamplerAllocation(sampler);
+ mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
@@ -1935,7 +1938,7 @@
void Context::samplerParameterf(GLuint sampler, GLenum pname, GLfloat param)
{
- mResourceManager->checkSamplerAllocation(sampler);
+ mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
@@ -1960,7 +1963,7 @@
GLint Context::getSamplerParameteri(GLuint sampler, GLenum pname)
{
- mResourceManager->checkSamplerAllocation(sampler);
+ mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
@@ -1985,7 +1988,7 @@
GLfloat Context::getSamplerParameterf(GLuint sampler, GLenum pname)
{
- mResourceManager->checkSamplerAllocation(sampler);
+ mResourceManager->checkSamplerAllocation(mImplementation.get(), sampler);
Sampler *samplerObject = getSampler(sampler);
ASSERT(samplerObject);
diff --git a/src/libANGLE/ResourceManager.cpp b/src/libANGLE/ResourceManager.cpp
index 5c5d907..7fb7109 100644
--- a/src/libANGLE/ResourceManager.cpp
+++ b/src/libANGLE/ResourceManager.cpp
@@ -20,7 +20,7 @@
namespace gl
{
-ResourceManager::ResourceManager(rx::GLImplFactory *factory) : mFactory(factory), mRefCount(1)
+ResourceManager::ResourceManager() : mRefCount(1)
{
}
@@ -86,13 +86,15 @@
}
// Returns an unused shader/program name
-GLuint ResourceManager::createShader(const gl::Limitations &rendererLimitations, GLenum type)
+GLuint ResourceManager::createShader(rx::GLImplFactory *factory,
+ const gl::Limitations &rendererLimitations,
+ GLenum type)
{
GLuint handle = mProgramShaderHandleAllocator.allocate();
if (type == GL_VERTEX_SHADER || type == GL_FRAGMENT_SHADER)
{
- mShaderMap[handle] = new Shader(this, mFactory, rendererLimitations, type, handle);
+ mShaderMap[handle] = new Shader(this, factory, rendererLimitations, type, handle);
}
else UNREACHABLE();
@@ -100,11 +102,11 @@
}
// Returns an unused program/shader name
-GLuint ResourceManager::createProgram()
+GLuint ResourceManager::createProgram(rx::GLImplFactory *factory)
{
GLuint handle = mProgramShaderHandleAllocator.allocate();
- mProgramMap[handle] = new Program(mFactory, this, handle);
+ mProgramMap[handle] = new Program(factory, this, handle);
return handle;
}
@@ -140,11 +142,11 @@
}
// Returns the next unused fence name, and allocates the fence
-GLuint ResourceManager::createFenceSync()
+GLuint ResourceManager::createFenceSync(rx::GLImplFactory *factory)
{
GLuint handle = mFenceSyncHandleAllocator.allocate();
- FenceSync *fenceSync = new FenceSync(mFactory->createFenceSync(), handle);
+ FenceSync *fenceSync = new FenceSync(factory->createFenceSync(), handle);
fenceSync->addRef();
mFenceSyncMap[handle] = fenceSync;
@@ -355,7 +357,7 @@
mRenderbufferMap[handle] = buffer;
}
-Buffer *ResourceManager::checkBufferAllocation(GLuint handle)
+Buffer *ResourceManager::checkBufferAllocation(rx::GLImplFactory *factory, GLuint handle)
{
if (handle == 0)
{
@@ -370,7 +372,7 @@
return bufferMapIt->second;
}
- Buffer *buffer = new Buffer(mFactory->createBuffer(), handle);
+ Buffer *buffer = new Buffer(factory->createBuffer(), handle);
buffer->addRef();
if (handleAllocated)
@@ -386,7 +388,9 @@
return buffer;
}
-Texture *ResourceManager::checkTextureAllocation(GLuint handle, GLenum type)
+Texture *ResourceManager::checkTextureAllocation(rx::GLImplFactory *factory,
+ GLuint handle,
+ GLenum type)
{
if (handle == 0)
{
@@ -401,7 +405,7 @@
return textureMapIt->second;
}
- Texture *texture = new Texture(mFactory, handle, type);
+ Texture *texture = new Texture(factory, handle, type);
texture->addRef();
if (handleAllocated)
@@ -417,7 +421,8 @@
return texture;
}
-Renderbuffer *ResourceManager::checkRenderbufferAllocation(GLuint handle)
+Renderbuffer *ResourceManager::checkRenderbufferAllocation(rx::GLImplFactory *factory,
+ GLuint handle)
{
if (handle == 0)
{
@@ -432,7 +437,7 @@
return renderbufferMapIt->second;
}
- Renderbuffer *renderbuffer = new Renderbuffer(mFactory->createRenderbuffer(), handle);
+ Renderbuffer *renderbuffer = new Renderbuffer(factory->createRenderbuffer(), handle);
renderbuffer->addRef();
if (handleAllocated)
@@ -448,7 +453,7 @@
return renderbuffer;
}
-Sampler *ResourceManager::checkSamplerAllocation(GLuint samplerHandle)
+Sampler *ResourceManager::checkSamplerAllocation(rx::GLImplFactory *factory, GLuint samplerHandle)
{
// Samplers cannot be created via Bind
if (samplerHandle == 0)
@@ -460,7 +465,7 @@
if (!sampler)
{
- sampler = new Sampler(mFactory, samplerHandle);
+ sampler = new Sampler(factory, samplerHandle);
mSamplerMap[samplerHandle] = sampler;
sampler->addRef();
}
diff --git a/src/libANGLE/ResourceManager.h b/src/libANGLE/ResourceManager.h
index baac331..9001315 100644
--- a/src/libANGLE/ResourceManager.h
+++ b/src/libANGLE/ResourceManager.h
@@ -34,19 +34,21 @@
class ResourceManager : angle::NonCopyable
{
public:
- explicit ResourceManager(rx::GLImplFactory *factory);
+ ResourceManager();
~ResourceManager();
void addRef();
void release();
GLuint createBuffer();
- GLuint createShader(const gl::Limitations &rendererLimitations, GLenum type);
- GLuint createProgram();
+ GLuint createShader(rx::GLImplFactory *factory,
+ const gl::Limitations &rendererLimitations,
+ GLenum type);
+ GLuint createProgram(rx::GLImplFactory *factory);
GLuint createTexture();
GLuint createRenderbuffer();
GLuint createSampler();
- GLuint createFenceSync();
+ GLuint createFenceSync(rx::GLImplFactory *factory);
void deleteBuffer(GLuint buffer);
void deleteShader(GLuint shader);
@@ -66,17 +68,17 @@
void setRenderbuffer(GLuint handle, Renderbuffer *renderbuffer);
- Buffer *checkBufferAllocation(GLuint handle);
- Texture *checkTextureAllocation(GLuint handle, GLenum type);
- Renderbuffer *checkRenderbufferAllocation(GLuint handle);
- Sampler *checkSamplerAllocation(GLuint samplerHandle);
+ Buffer *checkBufferAllocation(rx::GLImplFactory *factory, GLuint handle);
+ Texture *checkTextureAllocation(rx::GLImplFactory *factory, GLuint handle, GLenum type);
+ Renderbuffer *checkRenderbufferAllocation(rx::GLImplFactory *factory, GLuint handle);
+ Sampler *checkSamplerAllocation(rx::GLImplFactory *factory, GLuint samplerHandle);
bool isSampler(GLuint sampler);
private:
void createTextureInternal(GLuint handle);
- rx::GLImplFactory *mFactory;
+ ;
std::size_t mRefCount;
ResourceMap<Buffer> mBufferMap;
diff --git a/src/libANGLE/ResourceManager_unittest.cpp b/src/libANGLE/ResourceManager_unittest.cpp
index bb37ee5..ded2dc5 100644
--- a/src/libANGLE/ResourceManager_unittest.cpp
+++ b/src/libANGLE/ResourceManager_unittest.cpp
@@ -25,7 +25,7 @@
protected:
void SetUp() override
{
- mResourceManager = new ResourceManager(&mMockFactory);
+ mResourceManager = new ResourceManager();
}
void TearDown() override
@@ -41,7 +41,7 @@
{
EXPECT_CALL(mMockFactory, createTexture(_)).Times(1).RetiresOnSaturation();
- mResourceManager->checkTextureAllocation(1, GL_TEXTURE_2D);
+ mResourceManager->checkTextureAllocation(&mMockFactory, 1, GL_TEXTURE_2D);
GLuint newTexture = mResourceManager->createTexture();
EXPECT_NE(1u, newTexture);
}
@@ -50,7 +50,7 @@
{
EXPECT_CALL(mMockFactory, createBuffer()).Times(1).RetiresOnSaturation();
- mResourceManager->checkBufferAllocation(1);
+ mResourceManager->checkBufferAllocation(&mMockFactory, 1);
GLuint newBuffer = mResourceManager->createBuffer();
EXPECT_NE(1u, newBuffer);
}
@@ -59,9 +59,9 @@
{
EXPECT_CALL(mMockFactory, createRenderbuffer()).Times(1).RetiresOnSaturation();
- mResourceManager->checkRenderbufferAllocation(1);
+ mResourceManager->checkRenderbufferAllocation(&mMockFactory, 1);
GLuint newRenderbuffer = mResourceManager->createRenderbuffer();
EXPECT_NE(1u, newRenderbuffer);
}
-}
+} // anonymous namespace
diff --git a/src/tests/angle_end2end_tests.gypi b/src/tests/angle_end2end_tests.gypi
index d138b1c..f2e898f 100644
--- a/src/tests/angle_end2end_tests.gypi
+++ b/src/tests/angle_end2end_tests.gypi
@@ -73,6 +73,7 @@
'<(angle_path)/src/tests/gl_tests/VertexAttributeTest.cpp',
'<(angle_path)/src/tests/gl_tests/ViewportTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLContextCompatibilityTest.cpp',
+ '<(angle_path)/src/tests/egl_tests/EGLContextSharingTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLQueryContextTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLSanityCheckTest.cpp',
'<(angle_path)/src/tests/egl_tests/EGLSurfaceTest.cpp',
diff --git a/src/tests/egl_tests/EGLContextSharingTest.cpp b/src/tests/egl_tests/EGLContextSharingTest.cpp
new file mode 100644
index 0000000..fc66b79
--- /dev/null
+++ b/src/tests/egl_tests/EGLContextSharingTest.cpp
@@ -0,0 +1,87 @@
+//
+// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// EGLContextSharingTest.cpp:
+// Tests relating to shared Contexts.
+
+#include <gtest/gtest.h>
+
+#include <EGL/egl.h>
+#include <EGL/eglext.h>
+
+#include "test_utils/ANGLETest.h"
+#include "test_utils/angle_test_configs.h"
+
+using namespace angle;
+
+namespace
+{
+
+EGLBoolean SafeDestroyContext(EGLDisplay display, EGLContext &context)
+{
+ EGLBoolean result = EGL_TRUE;
+ if (context != EGL_NO_CONTEXT)
+ {
+ result = eglDestroyContext(display, context);
+ context = EGL_NO_CONTEXT;
+ }
+ return result;
+}
+
+class EGLContextSharingTest : public ANGLETest
+{
+ public:
+ EGLContextSharingTest() : mContexts{EGL_NO_CONTEXT, EGL_NO_CONTEXT}, mTexture(0) {}
+
+ void TearDown() override
+ {
+ glDeleteTextures(1, &mTexture);
+
+ EGLDisplay display = getEGLWindow()->getDisplay();
+
+ if (display != EGL_NO_DISPLAY)
+ {
+ for (auto &context : mContexts)
+ {
+ SafeDestroyContext(display, context);
+ }
+ }
+
+ ANGLETest::TearDown();
+ }
+
+ EGLContext mContexts[2];
+ GLuint mTexture;
+};
+
+// Tests that creating resources works after freeing the share context.
+TEST_P(EGLContextSharingTest, BindTextureAfterShareContextFree)
+{
+ EGLDisplay display = getEGLWindow()->getDisplay();
+ EGLConfig config = getEGLWindow()->getConfig();
+ EGLSurface surface = getEGLWindow()->getSurface();
+
+ const EGLint contextAttribs[] = {EGL_CONTEXT_CLIENT_VERSION,
+ getEGLWindow()->getClientMajorVersion(), EGL_NONE};
+
+ mContexts[0] = eglCreateContext(display, config, nullptr, contextAttribs);
+ ASSERT_EGL_SUCCESS();
+ ASSERT_TRUE(mContexts[0] != EGL_NO_CONTEXT);
+ mContexts[1] = eglCreateContext(display, config, mContexts[1], contextAttribs);
+ ASSERT_EGL_SUCCESS();
+ ASSERT_TRUE(mContexts[1] != EGL_NO_CONTEXT);
+
+ ASSERT_EGL_TRUE(SafeDestroyContext(display, mContexts[0]));
+ ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, mContexts[1]));
+ ASSERT_EGL_SUCCESS();
+
+ glGenTextures(1, &mTexture);
+ glBindTexture(GL_TEXTURE_2D, mTexture);
+ ASSERT_GL_NO_ERROR();
+}
+
+} // anonymous namespace
+
+ANGLE_INSTANTIATE_TEST(EGLContextSharingTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL());