[engine] dont double free surface texture interop objects. (#170284)

Fixes https://github.com/flutter/flutter/issues/152459

I suspect the double free is related to
https://github.com/flutter/flutter/issues/162147 .

With the Impeller backend, the second free is performed by the reactor
and comes only after the application has returned to the foreground.
This increaases the likelyhood that we nuke a handle that is in active
use by another part of the system.

Since SurfaceTexture:: detachFromGLContext is documented as releasing
the texture, we don't need to free it at all and can "leak" it on our
side.
This commit is contained in:
Jonah Williams 2025-06-10 09:25:12 -07:00 committed by GitHub
parent ee0eb22cdb
commit 2d30ce56fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 41 additions and 4 deletions

View File

@ -233,6 +233,9 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) {
}
void ReactorGLES::CollectHandle(HandleGLES handle) {
if (handle.IsDead()) {
return;
}
if (handle.untracked_id_.has_value()) {
LiveHandle live_handle(GLStorage{.integer = handle.untracked_id_.value()});
live_handle.pending_collection = true;

View File

@ -7,6 +7,7 @@
#include "flutter/impeller/renderer/backend/gles/texture_gles.h"
#include "flutter/testing/testing.h"
#include "gtest/gtest.h"
#include "impeller/base/validation.h"
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
@ -93,4 +94,26 @@ TEST_P(TextureGLESTest, Binds2DTexture) {
}
}
TEST_P(TextureGLESTest, Leak) {
TextureDescriptor desc;
desc.storage_mode = StorageMode::kDevicePrivate;
desc.size = {100, 100};
desc.format = PixelFormat::kR8G8B8A8UNormInt;
desc.type = TextureType::kTexture2D;
desc.sample_count = SampleCount::kCount1;
auto texture = GetContext()->GetResourceAllocator()->CreateTexture(desc);
ASSERT_TRUE(texture);
std::optional<GLuint> handle = TextureGLES::Cast(*texture).GetGLHandle();
EXPECT_TRUE(handle.has_value());
TextureGLES::Cast(*texture).Leak();
ScopedValidationDisable disable_validation;
handle = TextureGLES::Cast(*texture).GetGLHandle();
EXPECT_FALSE(handle.has_value());
}
} // namespace impeller::testing

View File

@ -16,6 +16,7 @@
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
#include "impeller/renderer/backend/gles/formats_gles.h"
#include "impeller/renderer/backend/gles/handle_gles.h"
namespace impeller {
@ -229,6 +230,10 @@ TextureGLES::~TextureGLES() {
}
}
void TextureGLES::Leak() {
handle_ = HandleGLES::DeadHandle();
}
// |Texture|
bool TextureGLES::IsValid() const {
return is_valid_;

View File

@ -102,6 +102,10 @@ class TextureGLES final : public Texture,
bool IsWrapped() const;
/// @brief Reset the internal texture state so that the reactor will not free
/// the associated handle.
void Leak();
std::optional<GLuint> GetFBO() const;
//----------------------------------------------------------------------------

View File

@ -55,6 +55,9 @@ void SurfaceTextureExternalTextureGLImpeller::ProcessFrame(
void SurfaceTextureExternalTextureGLImpeller::Detach() {
SurfaceTextureExternalTexture::Detach();
// Detach will collect the texture handle.
// See also: https://github.com/flutter/flutter/issues/152459
texture_->Leak();
texture_.reset();
}

View File

@ -52,10 +52,9 @@ void SurfaceTextureExternalTextureGLSkia::ProcessFrame(PaintContext& context,
void SurfaceTextureExternalTextureGLSkia::Detach() {
SurfaceTextureExternalTexture::Detach();
if (texture_name_ != 0) {
glDeleteTextures(1, &texture_name_);
texture_name_ = 0;
}
// Detach will collect the texture handle.
// See also: https://github.com/flutter/flutter/issues/152459
texture_name_ = 0;
}
} // namespace flutter