Commit 905ee082 by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Fix cleanup race condition on Context destroy

In Context::onDestroy(), e7b3fe21 had moved surface deletion first which down the line caused RendererVk::finish() to be called. bf7b95db however made surface deletion unnecessary, which means finish was never called. This commit adds an explicit finish in Context::onDestroy(). In truth, the wait is only necessary until all command buffers submitted for this particular context have finished. This optimization is deferred to a possible future work. Bug: angleproject:2811 Change-Id: I56e6c88d3b4a6ec73f70d80d7775a0c85be651ea Reviewed-on: https://chromium-review.googlesource.com/c/1302838 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org>
parent 448b99f1
...@@ -545,12 +545,15 @@ void Context::initialize() ...@@ -545,12 +545,15 @@ void Context::initialize()
egl::Error Context::onDestroy(const egl::Display *display) 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) if (mGLES1Renderer)
{ {
mGLES1Renderer->onDestroy(this, &mGLState); mGLES1Renderer->onDestroy(this, &mGLState);
} }
// Delete the Surface first to trigger a finish() in Vulkan.
ANGLE_TRY(releaseSurface(display)); ANGLE_TRY(releaseSurface(display));
for (auto fence : mFenceNVMap) for (auto fence : mFenceNVMap)
......
...@@ -1203,6 +1203,12 @@ angle::Result Renderer11::flush(Context11 *context11) ...@@ -1203,6 +1203,12 @@ angle::Result Renderer11::flush(Context11 *context11)
angle::Result Renderer11::finish(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()) if (!mSyncQuery.valid())
{ {
D3D11_QUERY_DESC queryDesc; D3D11_QUERY_DESC queryDesc;
......
...@@ -648,9 +648,20 @@ angle::Result Renderer9::finish(const gl::Context *context) ...@@ -648,9 +648,20 @@ angle::Result Renderer9::finish(const gl::Context *context)
} }
ANGLE_TRY_HR(context9, result, "Failed to get event query data"); ANGLE_TRY_HR(context9, result, "Failed to get event query data");
// Loop until the query completes // 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.
unsigned int attempt = 0; unsigned int attempt = 0;
while (result == S_FALSE)
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)
{ {
// Keep polling, but allow other threads to do something useful first // Keep polling, but allow other threads to do something useful first
ScheduleYield(); ScheduleYield();
...@@ -675,6 +686,8 @@ angle::Result Renderer9::finish(const gl::Context *context) ...@@ -675,6 +686,8 @@ angle::Result Renderer9::finish(const gl::Context *context)
freeEventQuery(query); freeEventQuery(query);
} }
ANGLE_TRY_HR(context9, result, "Failed to get event query data"); ANGLE_TRY_HR(context9, result, "Failed to get event query data");
QueryPerformanceCounter(&currentTime);
} }
freeEventQuery(query); freeEventQuery(query);
......
...@@ -193,7 +193,7 @@ class RendererVk : angle::NonCopyable ...@@ -193,7 +193,7 @@ class RendererVk : angle::NonCopyable
private: private:
// Number of semaphores for external entities to renderer to issue a wait, such as surface's // Number of semaphores for external entities to renderer to issue a wait, such as surface's
// image acquire. // image acquire.
static constexpr size_t kMaxExternalSemaphores = 8; static constexpr size_t kMaxExternalSemaphores = 64;
// Total possible number of semaphores a submission can wait on. +1 is for the semaphore // Total possible number of semaphores a submission can wait on. +1 is for the semaphore
// signaled in the last submission. // signaled in the last submission.
static constexpr size_t kMaxWaitSemaphores = kMaxExternalSemaphores + 1; static constexpr size_t kMaxWaitSemaphores = kMaxExternalSemaphores + 1;
......
...@@ -179,8 +179,7 @@ ...@@ -179,8 +179,7 @@
2635 WIN VULKAN : dEQP-EGL.functional.resize.surface_size.stretch_width = FAIL 2635 WIN VULKAN : dEQP-EGL.functional.resize.surface_size.stretch_width = FAIL
2635 WIN VULKAN : dEQP-EGL.functional.wide_color.pbuffer_8888_colorspace_default = FAIL 2635 WIN VULKAN : dEQP-EGL.functional.wide_color.pbuffer_8888_colorspace_default = FAIL
2716 WIN VULKAN : dEQP-EGL.functional.preserve_swap.no_preserve.* = FAIL 2716 WIN VULKAN : dEQP-EGL.functional.preserve_swap.no_preserve.* = FAIL
2811 WIN VULKAN : dEQP-EGL.functional.swap_buffers_with_damage.* = SKIP 2811 WIN VULKAN : dEQP-EGL.functional.swap_buffers_with_damage.resize* = SKIP
// Linux failures // Linux failures
2546 LINUX : dEQP-EGL.functional.color_clears.multi_context.gles1.rgba8888_pixmap = SKIP 2546 LINUX : dEQP-EGL.functional.color_clears.multi_context.gles1.rgba8888_pixmap = SKIP
......
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