From bcaa44592e84345ffd3b0190f7a5a7f40256d2f1 Mon Sep 17 00:00:00 2001 From: Kingtous Date: Mon, 3 Apr 2023 07:30:04 +0800 Subject: [PATCH] [Linux] fix: make textures thread-safe on linux (flutter/engine#40478) Make textures GHashTable thread-safe --- .../platform/linux/fl_texture_registrar.cc | 19 ++++++++++- .../linux/fl_texture_registrar_test.cc | 32 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) mode change 100644 => 100755 engine/src/flutter/shell/platform/linux/fl_texture_registrar.cc diff --git a/engine/src/flutter/shell/platform/linux/fl_texture_registrar.cc b/engine/src/flutter/shell/platform/linux/fl_texture_registrar.cc old mode 100644 new mode 100755 index 0851d22edd..d22e24cf32 --- a/engine/src/flutter/shell/platform/linux/fl_texture_registrar.cc +++ b/engine/src/flutter/shell/platform/linux/fl_texture_registrar.cc @@ -34,6 +34,9 @@ struct _FlTextureRegistrarImpl { // plugins. The keys are directly stored int64s. The values are stored // pointer to #FlTexture. This table is freed by the responder. GHashTable* textures; + + // The mutex guard to make `textures` thread-safe. + GMutex textures_mutex; }; static void fl_texture_registrar_impl_iface_init( @@ -57,21 +60,26 @@ static void engine_weak_notify_cb(gpointer user_data, self->engine = nullptr; // Unregister any textures. + g_mutex_lock(&self->textures_mutex); g_autoptr(GHashTable) textures = self->textures; self->textures = g_hash_table_new_full(g_direct_hash, g_direct_equal, nullptr, g_object_unref); g_hash_table_remove_all(textures); + g_mutex_unlock(&self->textures_mutex); } static void fl_texture_registrar_impl_dispose(GObject* object) { FlTextureRegistrarImpl* self = FL_TEXTURE_REGISTRAR_IMPL(object); + g_mutex_lock(&self->textures_mutex); g_clear_pointer(&self->textures, g_hash_table_unref); + g_mutex_unlock(&self->textures_mutex); if (self->engine != nullptr) { g_object_weak_unref(G_OBJECT(self->engine), engine_weak_notify_cb, self); self->engine = nullptr; } + g_mutex_clear(&self->textures_mutex); G_OBJECT_CLASS(fl_texture_registrar_impl_parent_class)->dispose(object); } @@ -93,8 +101,10 @@ static gboolean register_texture(FlTextureRegistrar* registrar, int64_t id = self->next_id++; if (fl_engine_register_external_texture(self->engine, id)) { fl_texture_set_id(texture, id); + g_mutex_lock(&self->textures_mutex); g_hash_table_insert(self->textures, GINT_TO_POINTER(id), g_object_ref(texture)); + g_mutex_unlock(&self->textures_mutex); return TRUE; } else { return FALSE; @@ -108,8 +118,11 @@ static gboolean register_texture(FlTextureRegistrar* registrar, static FlTexture* lookup_texture(FlTextureRegistrar* registrar, int64_t texture_id) { FlTextureRegistrarImpl* self = FL_TEXTURE_REGISTRAR_IMPL(registrar); - return reinterpret_cast( + g_mutex_lock(&self->textures_mutex); + FlTexture* texture = reinterpret_cast( g_hash_table_lookup(self->textures, GINT_TO_POINTER(texture_id))); + g_mutex_unlock(&self->textures_mutex); + return texture; } static gboolean mark_texture_frame_available(FlTextureRegistrar* registrar, @@ -135,10 +148,12 @@ static gboolean unregister_texture(FlTextureRegistrar* registrar, gboolean result = fl_engine_unregister_external_texture( self->engine, fl_texture_get_id(texture)); + g_mutex_lock(&self->textures_mutex); if (!g_hash_table_remove(self->textures, GINT_TO_POINTER(fl_texture_get_id(texture)))) { g_warning("Unregistering a non-existent texture %p", texture); } + g_mutex_unlock(&self->textures_mutex); return result; } @@ -155,6 +170,8 @@ static void fl_texture_registrar_impl_init(FlTextureRegistrarImpl* self) { self->next_id = 1; self->textures = g_hash_table_new_full(g_direct_hash, g_direct_equal, nullptr, g_object_unref); + // Initialize the mutex for textures. + g_mutex_init(&self->textures_mutex); } G_MODULE_EXPORT gboolean diff --git a/engine/src/flutter/shell/platform/linux/fl_texture_registrar_test.cc b/engine/src/flutter/shell/platform/linux/fl_texture_registrar_test.cc index 17bac5917d..7f9fdb413a 100644 --- a/engine/src/flutter/shell/platform/linux/fl_texture_registrar_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_texture_registrar_test.cc @@ -13,11 +13,13 @@ #include #include +#include static constexpr uint32_t kBufferWidth = 4u; static constexpr uint32_t kBufferHeight = 4u; static constexpr uint32_t kRealBufferWidth = 2u; static constexpr uint32_t kRealBufferHeight = 2u; +static constexpr uint64_t kThreadCount = 16u; G_DECLARE_FINAL_TYPE(FlTestRegistrarTexture, fl_test_registrar_texture, @@ -64,6 +66,14 @@ static FlTestRegistrarTexture* fl_test_registrar_texture_new() { g_object_new(fl_test_registrar_texture_get_type(), nullptr)); } +static void* add_mock_texture_to_registrar(void* pointer) { + g_return_val_if_fail(FL_TEXTURE_REGISTRAR(pointer), ((void*)NULL)); + FlTextureRegistrar* registrar = FL_TEXTURE_REGISTRAR(pointer); + g_autoptr(FlTexture) texture = FL_TEXTURE(fl_test_registrar_texture_new()); + fl_texture_registrar_register_texture(registrar, texture); + pthread_exit(NULL); +} + // Checks can make a mock registrar. TEST(FlTextureRegistrarTest, MockRegistrar) { g_autoptr(FlTexture) texture = FL_TEXTURE(fl_test_registrar_texture_new()); @@ -104,3 +114,25 @@ TEST(FlTextureRegistrarTest, MarkTextureFrameAvailable) { EXPECT_TRUE( fl_texture_registrar_mark_texture_frame_available(registrar, texture)); } + +// Test the textures can be accessed via multiple threads without +// synchronization issues. +TEST(FlTextureRegistrarTest, RegistrarRegisterTextureInMultipleThreads) { + g_autoptr(FlEngine) engine = make_mock_engine(); + g_autoptr(FlTextureRegistrar) registrar = fl_texture_registrar_new(engine); + pthread_t threads[kThreadCount]; + + for (uint64_t t = 0; t < kThreadCount; t++) { + EXPECT_EQ(pthread_create(&threads[t], NULL, add_mock_texture_to_registrar, + (void*)registrar), + 0); + } + for (uint64_t t = 0; t < kThreadCount; t++) { + pthread_join(threads[t], NULL); + }; + // Check the texture named from [1, threadCount]. + for (uint64_t t = 1; t <= kThreadCount; t++) { + EXPECT_TRUE(fl_texture_registrar_lookup_texture(registrar, (int64_t)t) != + NULL); + }; +}