Skip to content

[pigeon] fix instance manager test flake - #13303

Merged
auto-submit[bot] merged 4 commits into
flutter:mainfrom
tarrinneal:pigeon-instance-manager-test-flake
Oct 19, 2026
Merged

auto-submit[bot] merged 4 commits into
flutter:mainfrom
tarrinneal:pigeon-instance-manager-test-flake

Conversation

@tarrinneal

Copy link
Copy Markdown
Contributor

hopefully this solves the problem with this test.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Oct 17, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates instance_manager_test.dart to use a non-inlined helper function annotated with @pragma('vm:never-inline') for creating and adding a temporary instance, and replaces the fixed garbage collection call with a loop that repeatedly triggers GC. The review feedback suggests applying this same helper pattern to the neighboring addHostCreatedInstance test to prevent potential false positives caused by stack retention.

Comment on lines +123 to +128
@pragma('vm:never-inline')
void addTemporaryInstance() {
instanceManager.addDartCreatedInstance(
CopyableObject(pigeon_instanceManager: instanceManager),
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The @pragma('vm:never-inline') helper pattern introduced here is necessary to prevent stack retention and ensure the object is eligible for garbage collection. However, the neighboring test addHostCreatedInstance should not add finalizer to original object (lines 138–154) still uses the old pattern of setting the local variable to null and calling forceGC. Because of stack retention, the object in that test might not actually be garbage collected, which would cause the test to pass even if a finalizer was mistakenly added (a false positive). Consider applying the same helper function pattern with @pragma('vm:never-inline') to the addHostCreatedInstance test to ensure the object is actually garbage collected.

@bparrishMines bparrishMines left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 19, 2026
@auto-submit
auto-submit Bot merged commit 5d26c9b into flutter:main Oct 19, 2026
13 checks passed
bkonyi pushed a commit to bkonyi/flutter that referenced this pull request Oct 20, 2026
flutter/packages@10a178c...618afba

2026-10-20 [email protected] [pigeon] add version checking for
objective_c and jni dependencies (flutter/packages#13283)
2026-10-19 [email protected] [go_router_builder] Adopt
shared analyzer helpers from source_helper: ^1.4.0
(flutter/packages#13337)
2026-10-19 [email protected] [pigeon] fix instance manager test flake
(flutter/packages#13303)
2026-10-19 [email protected] [vector_graphics] Don't report a handled
loading error as unhandled (flutter/packages#13315)
2026-10-19 [email protected] [file_selector_macos] Drop deprecated
allowedFileTypes code path (flutter/packages#13302)
2026-10-19 [email protected] [shared_preferences_android] Update
androidx.datastore:datastore to 1.2.1 (flutter/packages#13334)
2026-10-19 [email protected] [in_app_purchase_storekit] Suppress
StoreKit 1 deprecation warnings (flutter/packages#13301)
2026-10-19 [email protected] [camera_avfoundation] Fix Swift analyzer
issue _captureSessionQueue (flutter/packages#13332)
2026-10-19 [email protected] [material_ui]
Unskip web tests that pass on CanvasKit and Skwasm: color_scheme_test,
elevated_button_test, filled_button_test, icon_button_test
(flutter/packages#13327)
2026-10-19 [email protected] [in_app_purchase_android] Update
to Play Billing Library 9.1.0 (flutter/packages#12781)
2026-10-19 [email protected] [webview_flutter_wkwebview] Require iOS 15
/ macOS 12 and drop deprecated SecTrustGetCertificateAtIndex code path
(flutter/packages#13294)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
suryaKommana2662 pushed a commit to suryaKommana2662/flutter that referenced this pull request Oct 29, 2026
flutter/packages@10a178c...618afba

2026-10-20 [email protected] [pigeon] add version checking for
objective_c and jni dependencies (flutter/packages#13283)
2026-10-19 [email protected] [go_router_builder] Adopt
shared analyzer helpers from source_helper: ^1.4.0
(flutter/packages#13337)
2026-10-19 [email protected] [pigeon] fix instance manager test flake
(flutter/packages#13303)
2026-10-19 [email protected] [vector_graphics] Don't report a handled
loading error as unhandled (flutter/packages#13315)
2026-10-19 [email protected] [file_selector_macos] Drop deprecated
allowedFileTypes code path (flutter/packages#13302)
2026-10-19 [email protected] [shared_preferences_android] Update
androidx.datastore:datastore to 1.2.1 (flutter/packages#13334)
2026-10-19 [email protected] [in_app_purchase_storekit] Suppress
StoreKit 1 deprecation warnings (flutter/packages#13301)
2026-10-19 [email protected] [camera_avfoundation] Fix Swift analyzer
issue _captureSessionQueue (flutter/packages#13332)
2026-10-19 [email protected] [material_ui]
Unskip web tests that pass on CanvasKit and Skwasm: color_scheme_test,
elevated_button_test, filled_button_test, icon_button_test
(flutter/packages#13327)
2026-10-19 [email protected] [in_app_purchase_android] Update
to Play Billing Library 9.1.0 (flutter/packages#12781)
2026-10-19 [email protected] [webview_flutter_wkwebview] Require iOS 15
/ macOS 12 and drop deprecated SecTrustGetCertificateAtIndex code path
(flutter/packages#13294)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected] on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD p: pigeon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants