Skip to content

Commit 11123f1

Browse files
authored
Added assert for opengles thread safety (flutter/engine#56585)
This assert would have blocked the following bug: flutter#158535 This shouldn't be landed until flutter/engine#56573 lands. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent d0cf81d commit 11123f1

File tree

2 files changed

+24
-0
lines changed

2 files changed

+24
-0
lines changed

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ ProcTableGLES::ProcTableGLES( // NOLINT(google-readability-function-size)
140140

141141
capabilities_ = std::make_shared<CapabilitiesGLES>(*this);
142142

143+
// This this will force glUseProgram to only be used on one thread in debug
144+
// builds to identify threading violations in the engine.
145+
UseProgram.enforce_one_thread = true;
146+
143147
is_valid_ = true;
144148
}
145149

engine/src/flutter/impeller/renderer/backend/gles/proc_table_gles.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
#define FLUTTER_IMPELLER_RENDERER_BACKEND_GLES_PROC_TABLE_GLES_H_
77

88
#include <functional>
9+
#include <mutex>
910
#include <string>
11+
#include <thread>
1012

1113
#include "flutter/fml/logging.h"
1214
#include "flutter/fml/mapping.h"
@@ -99,6 +101,14 @@ struct GLProc {
99101
///
100102
bool log_calls = false;
101103

104+
//----------------------------------------------------------------------------
105+
/// Whether the OpenGL call asserts it is only used from / one thread in
106+
/// IMPELLER_DEBUG builds.
107+
///
108+
/// This is used to block drawing calls from happening anywhere but the raster
109+
/// thread.
110+
bool enforce_one_thread = false;
111+
102112
//----------------------------------------------------------------------------
103113
/// @brief Call the GL function with the appropriate parameters. Lookup
104114
/// the documentation for the GL function being called to
@@ -118,6 +128,16 @@ struct GLProc {
118128
FML_LOG(IMPORTANT) << name
119129
<< BuildGLArguments(std::forward<Args>(args)...);
120130
}
131+
if (enforce_one_thread) {
132+
static std::thread::id allowed_thread;
133+
static std::once_flag flag;
134+
std::call_once(flag,
135+
[]() { allowed_thread = std::this_thread::get_id(); });
136+
FML_CHECK(std::this_thread::get_id() == allowed_thread)
137+
<< "This symbol is expected to be called from one thread, the raster "
138+
"thread. As of this addition, the design of the engine should be "
139+
"using non-raster threads only for uploading images.";
140+
}
121141
#endif // defined(IMPELLER_DEBUG) && !defined(NDEBUG)
122142
return function(std::forward<Args>(args)...);
123143
}

0 commit comments

Comments
 (0)