Commit bd276beb by Tim Van Patten Committed by Commit Bot

Vulkan: Don't wait on unflushed sync objects without a Context

The app "Car Parking Multiplayer" issues a ClientWaitSync() command without having already flushed the sync object and without an active context. We should return TIMEOUT immediately rather than attempting to wait on the sync object, since we can't flush it and it'll never be signalled. Bug: angleproject:5613 Bug: angleproject:5656 Test: MultithreadingTest.NoFlushNoContextReturnsTimeout Change-Id: Ieaf675ca9144f9c851c73b9ca399daaf4ed1cd0f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2693375 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCharlie Lao <cclao@google.com>
parent 7e990ef4
...@@ -139,12 +139,28 @@ angle::Result SyncHelper::clientWait(Context *context, ...@@ -139,12 +139,28 @@ angle::Result SyncHelper::clientWait(Context *context,
// We defer (ignore) flushes, so it's possible that the glFence's signal operation is pending // We defer (ignore) flushes, so it's possible that the glFence's signal operation is pending
// submission. // submission.
if ((flushCommands && contextVk) || usedInRecordedCommands()) if (contextVk)
{ {
ANGLE_TRY(contextVk->flushImpl(nullptr)); if (flushCommands || usedInRecordedCommands())
{
ANGLE_TRY(contextVk->flushImpl(nullptr));
}
}
else
{
if (!mUse.getSerial().valid())
{
// The sync object wasn't flushed before waiting, so the wait will always necessarily
// time out.
WARN() << "clientWaitSync called without flushing sync object and/or a valid context "
"active.";
*outResult = VK_TIMEOUT;
return angle::Result::Continue;
}
} }
// If timeout is zero, there's no need to wait, so return timeout already. // If timeout is zero, there's no need to wait, so return timeout already.
// Do this after (possibly) flushing, since some apps/tests/traces are relying on this behavior.
if (timeout == 0) if (timeout == 0)
{ {
*outResult = VK_TIMEOUT; *outResult = VK_TIMEOUT;
......
...@@ -33,6 +33,12 @@ class MultithreadingTest : public ANGLETest ...@@ -33,6 +33,12 @@ class MultithreadingTest : public ANGLETest
setConfigAlphaBits(8); setConfigAlphaBits(8);
} }
bool hasFenceSyncExtension() const
{
return IsEGLDisplayExtensionEnabled(getEGLWindow()->getDisplay(), "EGL_KHR_fence_sync");
}
bool hasGLSyncExtension() const { return IsGLExtensionEnabled("GL_OES_EGL_sync"); }
void runMultithreadedGLTest( void runMultithreadedGLTest(
std::function<void(EGLSurface surface, size_t threadIndex)> testBody, std::function<void(EGLSurface surface, size_t threadIndex)> testBody,
size_t threadCount) size_t threadCount)
...@@ -648,6 +654,43 @@ TEST_P(MultithreadingTestES3, MultithreadFenceTexImage) ...@@ -648,6 +654,43 @@ TEST_P(MultithreadingTestES3, MultithreadFenceTexImage)
mainThreadDraw(false); mainThreadDraw(false);
} }
// Test that waiting on a sync object that hasn't been flushed and without a current context returns
// TIMEOUT_EXPIRED or CONDITION_SATISFIED, but doesn't generate an error or crash.
TEST_P(MultithreadingTest, NoFlushNoContextReturnsTimeout)
{
ANGLE_SKIP_TEST_IF(!platformSupportsMultithreading());
ANGLE_SKIP_TEST_IF(!hasFenceSyncExtension() || !hasGLSyncExtension());
std::mutex mutex;
EGLWindow *window = getEGLWindow();
EGLDisplay dpy = window->getDisplay();
glClearColor(1.0f, 0.0f, 1.0f, 1.0f);
glClear(GL_COLOR_BUFFER_BIT);
EGLSyncKHR sync = eglCreateSyncKHR(dpy, EGL_SYNC_FENCE_KHR, nullptr);
EXPECT_NE(sync, EGL_NO_SYNC_KHR);
std::thread thread = std::thread([&]() {
std::lock_guard<decltype(mutex)> lock(mutex);
// Make sure there is no active context on this thread.
EXPECT_EGL_TRUE(eglMakeCurrent(dpy, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT));
EXPECT_EGL_SUCCESS();
// Don't wait forever to make sure the test terminates
constexpr GLuint64 kTimeout = 1'000'000'000; // 1 second
int result = eglClientWaitSyncKHR(dpy, sync, 0, kTimeout);
// We typically expect to get back TIMEOUT_EXPIRED since the sync object was never flushed.
// However, the OpenGL ES backend returns CONDITION_SATISFIED, which is also a passing
// result.
ASSERT_TRUE(result == EGL_TIMEOUT_EXPIRED_KHR || result == EGL_CONDITION_SATISFIED_KHR);
});
thread.join();
EXPECT_EGL_TRUE(eglDestroySyncKHR(dpy, sync));
}
// TODO(geofflang): Test sharing a program between multiple shared contexts on multiple threads // TODO(geofflang): Test sharing a program between multiple shared contexts on multiple threads
ANGLE_INSTANTIATE_TEST(MultithreadingTest, ANGLE_INSTANTIATE_TEST(MultithreadingTest,
......
...@@ -383,6 +383,9 @@ TEST_P(ParallelShaderCompileTestES31, LinkAndDispatchManyPrograms) ...@@ -383,6 +383,9 @@ TEST_P(ParallelShaderCompileTestES31, LinkAndDispatchManyPrograms)
// Suspectable to the flakyness of http://anglebug.com/3349. // Suspectable to the flakyness of http://anglebug.com/3349.
ANGLE_SKIP_TEST_IF(IsWindows() && IsD3D11()); ANGLE_SKIP_TEST_IF(IsWindows() && IsD3D11());
// TODO(http://anglebug.com/5656): Fails on Linux+Intel+OpenGL
ANGLE_SKIP_TEST_IF(IsLinux() && IsIntel() && IsOpenGL());
ANGLE_SKIP_TEST_IF(!ensureParallelShaderCompileExtensionAvailable()); ANGLE_SKIP_TEST_IF(!ensureParallelShaderCompileExtensionAvailable());
TaskRunner<ImageLoadStore> runner; TaskRunner<ImageLoadStore> runner;
......
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