Improve sync object related command handling.

gpu/command_buffer/service/gles2_cmd_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
@@ skipping 1617 matching lines @@
   void DoRenderbufferStorageMultisampleCHROMIUM(
       GLenum target, GLsizei samples, GLenum internalformat,
       GLsizei width, GLsizei height);
 
   // Handler for glRenderbufferStorageMultisampleEXT
   // (multisampled_render_to_texture).
   void DoRenderbufferStorageMultisampleEXT(
       GLenum target, GLsizei samples, GLenum internalformat,
       GLsizei width, GLsizei height);
 
+  // Wrapper for glFenceSync.
+  GLsync DoFenceSync(GLenum condition, GLbitfield flags);
+
   // Common validation for multisample extensions.
   bool ValidateRenderbufferStorageMultisample(GLsizei samples,
                                               GLenum internalformat,
                                               GLsizei width,
                                               GLsizei height);
 
   // Verifies that the currently bound multisample renderbuffer is valid
   // Very slow! Only done on platforms with driver bugs that return invalid
   // buffers under memory pressure
   bool VerifyMultisampleRenderbufferIntegrity(
@@ skipping 2161 matching lines @@
       RemoveTransformFeedback(client_ids[ii]);
     }
   }
 }
 
 void GLES2DecoderImpl::DeleteSyncHelper(GLuint sync) {
   GLsync service_id = 0;
   if (group_->GetSyncServiceId(sync, &service_id)) {
     glDeleteSync(service_id);
     group_->RemoveSyncId(sync);
-  } else {
+  } else if (sync != 0) {
     LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "glDeleteSync", "unknown sync");
   }
 }
 
 bool GLES2DecoderImpl::MakeCurrent() {
   DCHECK(surface_);
   if (!context_.get())
     return false;
 
   if (WasContextLost()) {
@@ skipping 11526 matching lines @@
   if (!program) {
     return error::kNoError;
   }
   GLuint service_id = program->service_id();
   glUniformBlockBinding(service_id, index, binding);
   return error::kNoError;
 }
 
 error::Error GLES2DecoderImpl::HandleClientWaitSync(
     uint32_t immediate_data_size, const void* cmd_data) {
+  const char* function_name = "glClientWaitSync";
   if (!unsafe_es3_apis_enabled())
     return error::kUnknownCommand;
   const gles2::cmds::ClientWaitSync& c =
       *static_cast<const gles2::cmds::ClientWaitSync*>(cmd_data);
   const GLuint sync = static_cast<GLuint>(c.sync);
   const GLbitfield flags = static_cast<GLbitfield>(c.flags);
   const GLuint64 timeout = c.timeout();
   typedef cmds::ClientWaitSync::Result Result;
   Result* result_dst = GetSharedMemoryAs<Result*>(
       c.result_shm_id, c.result_shm_offset, sizeof(*result_dst));
   if (!result_dst) {
     return error::kOutOfBounds;
   }
   if (*result_dst != GL_WAIT_FAILED) {
     return error::kInvalidArguments;
   }
   GLsync service_sync = 0;
   if (!group_->GetSyncServiceId(sync, &service_sync)) {
-    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "ClientWaitSync", "invalid sync");
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid sync");
     return error::kNoError;
   }
-  *result_dst = glClientWaitSync(service_sync, flags, timeout);
+  // Only allow |flags| to be GL_SYNC_FLUSH_COMMANDS_BIT to avoid infinite wait.
+  if (flags != GL_SYNC_FLUSH_COMMANDS_BIT) {

[piman, 2016/06/23 00:58:32]

Oh, what I was thinking is, add GL_SYNC_FLUSH_COMMANDS_BIT even if the user
didn't specify it.
It is very possible that the user did things right and is allowed to not force a
flush (technically we /could/ track and verify this), and so we shouldn't fail,
but if we don't trust the user to do it right, we should force a flush to make
sure we don't block forever.

Then, I don't think we necessarily need a limit on timeout, in the sense that
it'll wait no longer than glFinish, and we already allow glFinish.

[Zhenyao Mo, 2016/06/23 01:07:46]

Sounds good. Done.

+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid flags");
+    return error::kNoError;
+  }
+  // TODO(zmo): Should we put a limit on |timeout|?
+  GLenum status = glClientWaitSync(service_sync, flags, timeout);
+  switch (status) {
+    case GL_ALREADY_SIGNALED:
+    case GL_TIMEOUT_EXPIRED:
+    case GL_CONDITION_SATISFIED:
+      break;
+    case GL_WAIT_FAILED:
+      // Avoid leaking GL errors when using virtual contexts.
+      LOCAL_PEEK_GL_ERROR(function_name);
+      break;
+    default:
+      NOTREACHED();
+      break;
+  }
+  *result_dst = status;
   return error::kNoError;
 }
 
 error::Error GLES2DecoderImpl::HandleWaitSync(
     uint32_t immediate_data_size, const void* cmd_data) {
+  const char* function_name = "glWaitSync";
   if (!unsafe_es3_apis_enabled())
     return error::kUnknownCommand;
   const gles2::cmds::WaitSync& c =
       *static_cast<const gles2::cmds::WaitSync*>(cmd_data);
   const GLuint sync = static_cast<GLuint>(c.sync);
   const GLbitfield flags = static_cast<GLbitfield>(c.flags);
   const GLuint64 timeout = c.timeout();
   GLsync service_sync = 0;
   if (!group_->GetSyncServiceId(sync, &service_sync)) {
-    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, "WaitSync", "invalid sync");
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid sync");
+    return error::kNoError;
+  }
+  if (flags != 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid flags");
+    return error::kNoError;
+  }
+  if (timeout != GL_TIMEOUT_IGNORED) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid timeout");
     return error::kNoError;
   }
   glWaitSync(service_sync, flags, timeout);
   return error::kNoError;
 }
 
+GLsync GLES2DecoderImpl::DoFenceSync(GLenum condition, GLbitfield flags) {
+  const char* function_name = "glFenceSync";
+  if (condition != GL_SYNC_GPU_COMMANDS_COMPLETE) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_ENUM, function_name, "invalid condition");
+    return 0;
+  }
+  if (flags != 0) {
+    LOCAL_SET_GL_ERROR(GL_INVALID_VALUE, function_name, "invalid flags");
+    return 0;
+  }
+  return glFenceSync(condition, flags);
+}
+
 error::Error GLES2DecoderImpl::HandleGetInternalformativ(
     uint32_t immediate_data_size, const void* cmd_data) {
   if (!unsafe_es3_apis_enabled())
     return error::kUnknownCommand;
   const gles2::cmds::GetInternalformativ& c =
       *static_cast<const gles2::cmds::GetInternalformativ*>(cmd_data);
   GLenum target = static_cast<GLenum>(c.target);
   GLenum format = static_cast<GLenum>(c.format);
   GLenum pname = static_cast<GLenum>(c.pname);
   if (!validators_->render_buffer_target.IsValid(target)) {
@@ skipping 1369 matching lines @@
 }
 
 // Include the auto-generated part of this file. We split this because it means
 // we can easily edit the non-auto generated parts right here in this file
 // instead of having to edit some template or the code generator.
 #include "base/macros.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_autogen.h"
 
 }  // namespace gles2
 }  // namespace gpu