Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

blasten
Copy link

@blasten blasten commented Feb 15, 2022

Fixes flutter/flutter#98530

On Android, the external view embedder may post a task to the platform
thread, and wait until it completes if overlay surfaces must be released during
application startup.

However, the platform thread might be blocked when Dart is initializing.

bool needsLayer;
{
std::lock_guard lock(layers_mutex_);
needsLayer = available_layer_index_ >= layers_.size();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any risk that the layers_ list may be modified between this check and the time when the layer is used?

The lock may need to be held during the entire operation. Or it may need an approach such as:

  • with the lock held:
    • obtain a layer if available and remove it from layers_
  • if no layer was available, then create a layer
  • with the lock held:
    • set up the layer and insert it into layers_

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it in 117787a, so the lock is held during the entire operation. PTAL

DestroyLayersUnsafe(jni_facade);
}

void SurfacePool::DestroyLayersUnsafe(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using the suffix Locked for this API instead of Unsafe. This method is safe as long as the caller holds the lock.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@blasten blasten added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 17, 2022
@fluttergithubbot fluttergithubbot merged commit 0ca8e1a into flutter:main Feb 17, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 17, 2022
itsjustkevin pushed a commit to itsjustkevin/engine that referenced this pull request Feb 28, 2022
itsjustkevin added a commit that referenced this pull request Feb 28, 2022
* Conditionally call FlutterViewDestroyOverlaySurfaces (#31464)

* Fix PhysicalShapeLayer paint bounds with clipping disabled (#31656)

* Fix PhysicalShapeLayer paint bounds with clipping disabled

* Add missing SkRect initialization

* 'add branch flutter-2.8-candidate.16 to enabled_branches in .ci.yaml'

* remove branch ref

Co-authored-by: Emmanuel Garcia <[email protected]>
Co-authored-by: Matej Knopp <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential deadlock in application startup
3 participants