From 59ac63d3e112e5e38d08737f04fa16195b28f6b9 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 (#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/lib/ui/painting/single_frame_codec.cc b/lib/ui/painting/single_frame_codec.cc index d799e31c36e981..44361f583a583c 100644 --- a/lib/ui/painting/single_frame_codec.cc +++ b/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));