Skip to content

Commit ee9f5f2

Browse files
authored
feat(texture): Tear down external textures on the render thread (#182)
1 parent 972cc1c commit ee9f5f2

6 files changed

Lines changed: 97 additions & 7 deletions

flutter/shell/platform/tizen/flutter_tizen.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
330330
int64_t texture_id,
331331
void (*callback)(void* user_data),
332332
void* user_data) {
333-
TextureRegistrarFromHandle(texture_registrar)->UnregisterTexture(texture_id);
333+
TextureRegistrarFromHandle(texture_registrar)
334+
->UnregisterTexture(texture_id, callback, user_data);
334335
}
335336

336337
bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

flutter/shell/platform/tizen/flutter_tizen_engine.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,29 @@ bool FlutterTizenEngine::MarkExternalTextureFrameAvailable(int64_t texture_id) {
412412
engine_, texture_id) == kSuccess);
413413
}
414414

415+
void FlutterTizenEngine::PostRenderThreadTask(std::function<void()> task) {
416+
if (!engine_) {
417+
// The engine is shutting down (or never started); posting is unsafe.
418+
// Run inline so the task's cleanup still happens rather than leaking.
419+
task();
420+
return;
421+
}
422+
auto* heap_task = new std::function<void()>(std::move(task));
423+
auto callback = [](void* data) {
424+
auto* fn = static_cast<std::function<void()>*>(data);
425+
(*fn)();
426+
delete fn;
427+
};
428+
if (embedder_api_.PostRenderThreadTask(engine_, callback, heap_task) !=
429+
kSuccess) {
430+
// The engine began shutting down between the IsRunning() check above and
431+
// this call, so the render thread task runner is no longer available.
432+
// Run the task inline (as the trampoline would have) so its cleanup
433+
// still happens rather than leaking heap_task.
434+
callback(heap_task);
435+
}
436+
}
437+
415438
void FlutterTizenEngine::UpdateAccessibilityFeatures(bool invert_colors,
416439
bool high_contrast) {
417440
int32_t flags = 0;

flutter/shell/platform/tizen/flutter_tizen_engine.h

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

9+
#include <functional>
910
#include <memory>
1011

1112
#include "flutter/shell/platform/common/accessibility_bridge.h"
@@ -171,6 +172,14 @@ class FlutterTizenEngine {
171172
// given |texture_id|.
172173
bool MarkExternalTextureFrameAvailable(int64_t texture_id);
173174

175+
// Posts |task| onto the Flutter render (raster) thread. May be called from
176+
// any thread while the engine is running. Used to tear down external-texture
177+
// GPU resources on the thread that owns the GL context, after any in-flight
178+
// frame callback for that texture has completed. If the engine is not
179+
// running, or posting fails because shutdown began concurrently, |task|
180+
// runs inline instead.
181+
void PostRenderThreadTask(std::function<void()> task);
182+
174183
// Dispatch accessibility action back to the Flutter framework.
175184
void DispatchAccessibilityAction(uint64_t target,
176185
FlutterSemanticsAction action,

flutter/shell/platform/tizen/flutter_tizen_texture_registrar.cc

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@
44

55
#include "flutter_tizen_texture_registrar.h"
66

7-
#include <iostream>
7+
#include <memory>
88
#include <mutex>
99

1010
#include "flutter/shell/platform/tizen/external_texture.h"
1111
#include "flutter/shell/platform/tizen/external_texture_surface_egl.h"
1212
#include "flutter/shell/platform/tizen/flutter_tizen_engine.h"
1313
#include "flutter/shell/platform/tizen/logger.h"
14+
#include "flutter/shell/platform/tizen/tizen_renderer_gl.h"
1415

1516
namespace flutter {
1617

@@ -67,16 +68,66 @@ int64_t FlutterTizenTextureRegistrar::RegisterTexture(
6768
return texture_id;
6869
}
6970

70-
bool FlutterTizenTextureRegistrar::UnregisterTexture(int64_t texture_id) {
71+
bool FlutterTizenTextureRegistrar::UnregisterTexture(int64_t texture_id,
72+
void (*callback)(void*),
73+
void* user_data) {
74+
std::unique_ptr<ExternalTexture> texture;
7175
{
7276
std::lock_guard<std::mutex> lock(map_mutex_);
7377
auto iter = textures_.find(texture_id);
7478
if (iter == textures_.end()) {
7579
return false;
7680
}
81+
// Remove from the map first so no *new* PopulateGLTexture() lookup can
82+
// find this texture. A PopulateGLTexture() already in flight on the render
83+
// thread may still hold a raw pointer to it (it drops map_mutex_ before
84+
// using the texture), so destruction is deferred to a render-thread task
85+
// below.
86+
texture = std::move(iter->second);
7787
textures_.erase(iter);
7888
}
79-
return engine_->UnregisterExternalTexture(texture_id);
89+
90+
// Destroy the texture on the render thread rather than here on the calling
91+
// (platform) thread. The engine runs this task only after any in-flight
92+
// frame callback on the render thread has completed, so it cannot race with
93+
// a PopulateGLTexture() that is still reading this texture. Tearing it down
94+
// on the platform thread instead would let the backing TBM buffer be freed
95+
// while the render thread still has it mapped/referenced -> heap corruption
96+
// and the "tbm_bo_free ... lock_cnt" crash. Running on the render thread
97+
// also puts glDeleteTextures on the thread that owns the GL context.
98+
//
99+
// The completion |callback| (used by webview_flutter_lwe to free its TBM
100+
// buffer pool) is likewise invoked only after the render thread is done with
101+
// the texture, giving the plugin a correct "safe to free" signal.
102+
FlutterTizenEngine* engine = engine_;
103+
auto* gl_renderer = dynamic_cast<TizenRendererGL*>(engine->renderer());
104+
// shared_ptr (not unique_ptr): PostRenderThreadTask takes a copyable
105+
// std::function, so the captured owner must be copyable.
106+
std::shared_ptr<ExternalTexture> tex(std::move(texture));
107+
engine->PostRenderThreadTask(
108+
[engine, gl_renderer, texture_id, tex, callback, user_data]() mutable {
109+
// On the render thread, make the render context current so
110+
// glDeleteTextures in the texture's destructor targets the correct
111+
// context (the engine does not guarantee a current context when
112+
// running posted tasks).
113+
//
114+
// If the engine has shut down between PostRenderThreadTask() being
115+
// called and this task running, PostRenderThreadTask() runs the task
116+
// inline and the renderer may already be destroyed, so skip
117+
// OnMakeCurrent() to avoid touching a dangling GL context/renderer.
118+
// UnregisterExternalTexture() stays unconditional: it is null-safe at
119+
// the embedder boundary (a torn-down engine handle just yields an
120+
// error) and is the registrar's core teardown step.
121+
if (engine->IsRunning() && gl_renderer) {
122+
gl_renderer->OnMakeCurrent();
123+
}
124+
tex.reset();
125+
engine->UnregisterExternalTexture(texture_id);
126+
if (callback) {
127+
callback(user_data);
128+
}
129+
});
130+
return true;
80131
}
81132

82133
bool FlutterTizenTextureRegistrar::MarkTextureFrameAvailable(

flutter/shell/platform/tizen/flutter_tizen_texture_registrar.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,14 @@ class FlutterTizenTextureRegistrar {
2929

3030
// Attempts to unregister the texture identified by |texture_id|.
3131
//
32-
// Returns true if the texture was successfully unregistered.
33-
bool UnregisterTexture(int64_t texture_id);
32+
// The texture's GPU resources are torn down asynchronously on the render
33+
// thread (after any in-flight frame callback for it has completed); the
34+
// optional |callback| (invoked with |user_data|) fires once that teardown
35+
// is done. Returns true if the texture was found and scheduled for
36+
// unregistration.
37+
bool UnregisterTexture(int64_t texture_id,
38+
void (*callback)(void* user_data),
39+
void* user_data);
3440

3541
// Notifies the engine about a new frame being available.
3642
//

flutter/shell/platform/tizen/flutter_tizen_texture_registrar_unittests.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ TEST_F(FlutterTizenTextureRegistrarTest, RegisterUnregisterTexture) {
9898
EXPECT_TRUE(registrar.MarkTextureFrameAvailable(texture_id));
9999
EXPECT_TRUE(mark_frame_available_called);
100100

101-
EXPECT_TRUE(registrar.UnregisterTexture(texture_id));
101+
EXPECT_TRUE(registrar.UnregisterTexture(texture_id, nullptr, nullptr));
102102
EXPECT_TRUE(unregister_called);
103103
}
104104

0 commit comments

Comments
 (0)