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

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Mar 16, 2022

related issue: flutter/flutter#95751

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

Copy link
Contributor

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Per the linked issue, @iskakaushik is going to take a shot at disabling these. Let's hold on disabling these just yet.

@iskakaushik
Copy link
Contributor

@chinmaygarde, I tried to reproduce this for a while today. Found a couple of data races:

Let us disable this for now. I'll re-investigate when I have more cycles.

DestroyShell(std::move(shell));
}

// TODO(https://github.com/flutter/flutter/issues/95751): Disabled due to
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please file a new issue to re-enable these tests and link it here? That way we still have an open issue to re-enable the tests. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@iskakaushik iskakaushik left a comment

Choose a reason for hiding this comment

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

LGTM

@ColdPaleLight
Copy link
Member Author

Per the linked issue, @iskakaushik is going to take a shot at disabling these. Let's hold on disabling these just yet.

(The requested change has been addressed)

@ColdPaleLight ColdPaleLight dismissed chinmaygarde’s stale review March 17, 2022 10:50

The requested change has been addressed

@ColdPaleLight ColdPaleLight 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 Mar 17, 2022
@fluttergithubbot fluttergithubbot merged commit 3a954ba into flutter:main Mar 17, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 17, 2022
zanderso pushed a commit to flutter/flutter that referenced this pull request Mar 17, 2022
* 7fe613a Roll Dart SDK from 5bc905e69609 to ad5a250935d5 (4 revisions) (flutter/engine#32079)

* 3a954ba Disable several tests due to flakiness (flutter/engine#32059)

* 05bba9b [web] Log all goldctl commands (flutter/engine#32072)

* 86b2c8d Roll Skia from 9301fe3779bb to 02ebd1a23381 (4 revisions) (flutter/engine#32067)

* abb1bce Roll Fuchsia Linux SDK from mVqiTwaVa... to RAyopISUl... (flutter/engine#32069)

* 481f4bc Roll Fuchsia Mac SDK from vWlaMIVkM... to -JEG0j8mn... (flutter/engine#32077)

* 560243c Roll Dart SDK from ad5a250935d5 to 5168cdd236a0 (1 revision) (flutter/engine#32081)

* e2e6b49 Roll Skia from 02ebd1a23381 to 5e035c66da3c (19 revisions) (flutter/engine#32082)

* e4056c5 ensure _futurize does not leak uncaught errors into the zone (flutter/engine#32070)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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.

4 participants