Skip to content

Commit

Permalink
Vulkan: Refix cleanup race condition on Context destroy
Browse files Browse the repository at this point in the history
This partially reverts commit 905ee08
due to regression caused on startup time.

Instead of calling finish before destroying the context, the objects in
the Vulkan backend are `release()`ed instead of `destroy()`ed, so they
will be kept alive for the duration of current work.

Bug: 904846
Change-Id: Ia774470666c4c0d4c1ddc348f685d621243de204
Reviewed-on: https://chromium-review.googlesource.com/c/1333969
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
  • Loading branch information
ShabbyX authored and Commit Bot committed Nov 14, 2018
1 parent 092481a commit fca8fd6
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 33 deletions.
4 changes: 0 additions & 4 deletions src/libANGLE/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -545,10 +545,6 @@ void Context::initialize()

egl::Error Context::onDestroy(const egl::Display *display)
{
// Trigger a finish() to make sure resources are not in use upon destruction. Particularly
// necessary for Vulkan.
finish();

if (mGLES1Renderer)
{
mGLES1Renderer->onDestroy(this, &mGLState);
Expand Down
6 changes: 0 additions & 6 deletions src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1203,12 +1203,6 @@ angle::Result Renderer11::flush(Context11 *context11)

angle::Result Renderer11::finish(Context11 *context11)
{
// If device is lost, there is nothing to finish. This is called on context destroy.
if (!mDevice)
{
return angle::Result::Continue();
}

if (!mSyncQuery.valid())
{
D3D11_QUERY_DESC queryDesc;
Expand Down
17 changes: 2 additions & 15 deletions src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,20 +648,9 @@ angle::Result Renderer9::finish(const gl::Context *context)
}
ANGLE_TRY_HR(context9, result, "Failed to get event query data");

// Loop until the query completes. A bug has been observed where the query result is always
// false, so we loop for a maximum of 100ms.
// Loop until the query completes
unsigned int attempt = 0;

LARGE_INTEGER timerFrequency;
QueryPerformanceFrequency(&timerFrequency);

LARGE_INTEGER startTime;
QueryPerformanceCounter(&startTime);
LARGE_INTEGER currentTime = startTime;

// Note: timerFrequency is ticks in one second
const LONGLONG kMaxWaitTicks = timerFrequency.QuadPart / 10;
while (result == S_FALSE && (currentTime.QuadPart - startTime.QuadPart) < kMaxWaitTicks)
while (result == S_FALSE)
{
// Keep polling, but allow other threads to do something useful first
ScheduleYield();
Expand All @@ -686,8 +675,6 @@ angle::Result Renderer9::finish(const gl::Context *context)
freeEventQuery(query);
}
ANGLE_TRY_HR(context9, result, "Failed to get event query data");

QueryPerformanceCounter(&currentTime);
}

freeEventQuery(query);
Expand Down
4 changes: 2 additions & 2 deletions src/libANGLE/renderer/vulkan/FramebufferVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ void FramebufferVk::destroy(const gl::Context *context)
RendererVk *renderer = contextVk->getRenderer();
mFramebuffer.release(renderer);

mReadPixelBuffer.destroy(contextVk->getDevice());
mBlitPixelBuffer.destroy(contextVk->getDevice());
mReadPixelBuffer.release(renderer);
mBlitPixelBuffer.release(renderer);
}

angle::Result FramebufferVk::discard(const gl::Context *context,
Expand Down
13 changes: 7 additions & 6 deletions src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,16 @@ VertexArrayVk::~VertexArrayVk()

void VertexArrayVk::destroy(const gl::Context *context)
{
VkDevice device = vk::GetImpl(context)->getRenderer()->getDevice();
RendererVk *renderer = vk::GetImpl(context)->getRenderer();

for (vk::DynamicBuffer &buffer : mCurrentArrayBufferConversion)
{
buffer.destroy(device);
buffer.release(renderer);
}
mDynamicVertexData.destroy(device);
mDynamicIndexData.destroy(device);
mTranslatedByteIndexData.destroy(device);
mLineLoopHelper.destroy(device);
mDynamicVertexData.release(renderer);
mDynamicIndexData.release(renderer);
mTranslatedByteIndexData.release(renderer);
mLineLoopHelper.release(renderer);
}

angle::Result VertexArrayVk::streamIndexData(ContextVk *contextVk,
Expand Down
5 changes: 5 additions & 0 deletions src/libANGLE/renderer/vulkan/vk_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,11 @@ angle::Result LineLoopHelper::streamIndices(ContextVk *contextVk,
return angle::Result::Continue();
}

void LineLoopHelper::release(RendererVk *renderer)
{
mDynamicIndexBuffer.release(renderer);
}

void LineLoopHelper::destroy(VkDevice device)
{
mDynamicIndexBuffer.destroy(device);
Expand Down
1 change: 1 addition & 0 deletions src/libANGLE/renderer/vulkan/vk_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ class LineLoopHelper final : angle::NonCopyable
VkBuffer *bufferHandleOut,
VkDeviceSize *bufferOffsetOut);

void release(RendererVk *renderer);
void destroy(VkDevice device);

static void Draw(uint32_t count, CommandBuffer *commandBuffer);
Expand Down

0 comments on commit fca8fd6

Please sign in to comment.