Skip to content

Commit

Permalink
Guard WaitOnGpu with extra OpenGL checks.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 592707594
  • Loading branch information
MediaPipe Team authored and copybara-github committed Dec 21, 2023
1 parent 835ee5e commit cfb4465
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
1 change: 1 addition & 0 deletions mediapipe/gpu/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ cc_library(
"@com_google_absl//absl/memory",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings:str_format",
"@com_google_absl//absl/synchronization",
] + select({
"//conditions:default": [],
Expand Down
33 changes: 33 additions & 0 deletions mediapipe/gpu/gl_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "absl/log/absl_log.h"
#include "absl/memory/memory.h"
#include "absl/status/status.h"
#include "absl/strings/str_format.h"
#include "absl/synchronization/mutex.h"
#include "mediapipe/framework/port.h" // IWYU pragma: keep
#include "mediapipe/framework/port/ret_check.h"
Expand Down Expand Up @@ -650,13 +651,45 @@ class GlSyncWrapper {
// TODO: do something if the wait fails?
}

// This method exists only for investigation purposes to distinguish stack
// traces: external vs. internal context.
// TODO: remove after glWaitSync crashes are resolved.
void WaitOnGpuExternalContext() { glWaitSync(sync_, 0, GL_TIMEOUT_IGNORED); }

void WaitOnGpu() {
if (!sync_) return;
// WebGL2 specifies a waitSync call, but since cross-context
// synchronization is not supported, it's actually a no-op. Firefox prints
// a warning when it's called, so let's just skip the call. See
// b/184637485 for details.
#ifndef __EMSCRIPTEN__

if (!GlContext::IsAnyContextCurrent()) {
// glWaitSync must be called on with some context current. Doing the
// opposite doesn't necessarily result in a crash or GL error. Hence,
// just logging an error and skipping the call.
ABSL_LOG_FIRST_N(ERROR, 1)
<< "An attempt to wait for a sync without any context current.";
return;
}

auto context = GlContext::GetCurrent();
if (context == nullptr) {
// This can happen when WaitOnGpu is invoked on an external context,
// created by other than GlContext::Create means.
WaitOnGpuExternalContext();
return;
}

// GlContext::ShouldUseFenceSync guards creation of sync objects, so this
// CHECK should never fail if clients use MediaPipe APIs in an intended way.
// TODO: remove after glWaitSync crashes are resolved.
ABSL_CHECK(context->ShouldUseFenceSync()) << absl::StrFormat(
"An attempt to wait for a sync when it should not be used. (OpenGL "
"Version "
"%d.%d)",
context->gl_major_version(), context->gl_minor_version());

glWaitSync(sync_, 0, GL_TIMEOUT_IGNORED);
#endif
}
Expand Down
5 changes: 5 additions & 0 deletions mediapipe/gpu/gl_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ typedef std::function<void()> GlVoidFunction;
typedef std::function<absl::Status()> GlStatusFunction;

class GlContext;
// TODO: remove after glWaitSync crashes are resolved.
class GlSyncWrapper;

// Generic interface for synchronizing access to a shared resource from a
// different context. This is an abstract class to keep users from
Expand Down Expand Up @@ -329,6 +331,9 @@ class GlContext : public std::enable_shared_from_this<GlContext> {
SyncTokenTypeForTest type);

private:
// TODO: remove after glWaitSync crashes are resolved.
friend GlSyncWrapper;

GlContext();

bool ShouldUseFenceSync() const;
Expand Down

0 comments on commit cfb4465

Please sign in to comment.