From 430bed2f4fc613d3a037ae0087bc95c25a444f7b Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 31 Jul 2019 14:58:44 -0700 Subject: [PATCH] Ensure that the SingleFrameCodec stays alive until the ImageDecoder invokes its callback (flutter/engine#10297) SingleFrameCodec had been holding a persistent handle to the peer Dart object while waiting for the decoder callback. However, Dart code could call Codec.dispose during that interval, which would cause the Dart peer to drop its reference to the SingleFrameCodec and potentially delete the SingleFrameCodec. Fixes https://github.com/flutter/flutter/issues/36631 --- .../lib/ui/painting/single_frame_codec.cc | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/engine/src/flutter/lib/ui/painting/single_frame_codec.cc b/engine/src/flutter/lib/ui/painting/single_frame_codec.cc index d799e31c36..44361f583a 100644 --- a/engine/src/flutter/lib/ui/painting/single_frame_codec.cc +++ b/engine/src/flutter/lib/ui/painting/single_frame_codec.cc @@ -50,18 +50,18 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { return tonic::ToDart("Image decoder not available."); } - auto raw_codec_wrapper = new DartPersistentValue( - dart_state, Dart_HandleFromWeakPersistent(dart_wrapper())); + // The SingleFrameCodec must be deleted on the UI thread. Allocate a RefPtr + // on the heap to ensure that the SingleFrameCodec remains alive until the + // decoder callback is invoked on the UI thread. The callback can then + // drop the reference. + fml::RefPtr* raw_codec_ref = + new fml::RefPtr(this); - // We dont want to to put the raw codec in a lambda capture because we have - // to mutate (i.e destroy) it in the callback. Using MakeCopyable will create - // a shared pointer for the captures which can be destroyed on any thread. But - // we have to ensure that the DartPersistentValue is only destroyed on the UI - // thread. - decoder->Decode(descriptor_, [raw_codec_wrapper](auto image) { - std::unique_ptr codec_wrapper(raw_codec_wrapper); + decoder->Decode(descriptor_, [raw_codec_ref](auto image) { + std::unique_ptr> codec_ref(raw_codec_ref); + fml::RefPtr codec(std::move(*codec_ref)); - auto state = codec_wrapper->dart_state().lock(); + auto state = codec->pending_callbacks_.front().dart_state().lock(); if (!state) { // This is probably because the isolate has been terminated before the @@ -72,9 +72,6 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { tonic::DartState::Scope scope(state.get()); - SingleFrameCodec* codec = tonic::DartConverter::FromDart( - codec_wrapper->value()); - if (image.get()) { auto canvas_image = fml::MakeRefCounted(); canvas_image->set_image(std::move(image));