-
Notifications
You must be signed in to change notification settings - Fork 6k
Ensure that Picture::RasterizeToImage destroys Dart persistent values on the UI thread #8182
Conversation
… on the UI thread The DartPersistentValue used to hold the image callback is tied to a Dart isolate. Destructing the DartPersistentValue requires entering the isolate and must be done on the UI thread. Fixes flutter/flutter#29379
| auto* raw_dart_image = tonic::ToDart(std::move(dart_image)); | ||
|
|
||
| // All done! | ||
| tonic::DartInvoke(image_callback->Get(), {raw_dart_image}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect that the std::unique_ptr to be destructed here after that image_callback is no longer used, which means destruction in the ui thread. Is it not the case? @chinmaygarde
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the old implementation, the std::unique_ptr was being captured into a ref counted lambda by MakeCopyable.
The MakeCopyable lambda was then being passed through the GPU thread to the UI thread. Either the GPU thread or the UI thread could drop the last reference to the lambda. If the GPU thread destroyed the lambda, then the captured image_callback would be destructed on the GPU thread, causing a failure.
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
…t values on the UI thread (flutter/engine#8182)
… on the UI thread (flutter#8182) The DartPersistentValue used to hold the image callback is tied to a Dart isolate. Destructing the DartPersistentValue requires entering the isolate and must be done on the UI thread. Fixes flutter/flutter#29379
…t values on the UI thread (flutter#8182)" This reverts commit 001573f.
…t values on the UI thread (flutter#8182)" This reverts commit 001573f.
I've faced the same problem related to this vm shutdown: "Unable to Enter Isolate : Multiple mutators entering an isolate / Dart VM is shutting down" in dart_api_impl.cc from Dart_EnterIsolate() function. And I digged some code related to dart_persistent_value.cc, dart_isolate_scope.cc, dart_api_impl.cc, but still don't know why "Destructing the DartPersistentValue requires entering the isolate and must be done on the UI thread", May I ask you why UI Thread? |
|
If I created DartPersistentValue's unique_ptr in io task runner: Then should I manually make sure that DartPersistentValue's unique_ptr is destructed in io thread or also ui thread? And not using the ref count logic inside MakeCopyable lambda. |
The DartPersistentValue used to hold the image callback is tied to a
Dart isolate. Destructing the DartPersistentValue requires entering
the isolate and must be done on the UI thread.
Fixes flutter/flutter#29379