Fix crash on gma::Initialize without a Firebase App #1320

AlmostMatt · 2023-05-11T21:17:25Z

Description

Resolves a crash on Android when calling gma::Initialize without a firebase App (#1247)

The crash occured because util::FindClass was called before util had been initialized. The fix is to reorder some lines so that util is initialized before calling util::FindClass.

Also updated existing tests to wait for the initialization future before calling terminate to resolve some flaky test failures.

Testing

Added a new integration test case to initialize GMA without a Firebase App.
Ran integration tests locally on a real device and also on the github runners:
https://github.com/firebase/firebase-cpp-sdk/actions/runs/5731312895

Type of Change

Place an x the applicable box:

Bug fix. Add the issue # below if applicable.
New feature. A non-breaking change which adds functionality.
Other, such as a build process or documentation change.

Notes

Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
Read the contribution guidelines CONTRIBUTING.md.
Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

github-actions · 2023-05-11T21:21:18Z

Integration test with FLAKINESS (succeeded after retry)

Requested by @AlmostMatt on commit 18367c0
Last updated: Wed Aug 2 10:36 PDT 2023
View integration test log & download artifacts

Failures	Configs
gma	[TEST] [FLAKINESS] [Android] [1/3 os: windows] [1/2 android_device: android_target] (1 failed tests) FirebaseGmaTest.TestAdViewLoadAdEmptyAdRequest

Add flaky tests to go/fpl-cpp-flake-tracker

jonsimantov

Should we get this PR merged in? (With the release note moved to the next version, that is.)

gma/src/android/gma_android.cc

gma/integration_test/src/integration_test.cc

…1419) * iOS: Remove Analytics dependency from GMA integration test (#1386) * Try using CoreOnly instead of Analytics cocoapod for tests. * Update readme. * Add stub workflow for updating feature branches. * Add script to merge main into all active feature branches on a regular schedule. (#1394) * Add workflow for automatically updating feature branches weekly. * Corrected filename, and added inputs. * Remove trailing spaces. * Fix workflow. * Fix workflow name. * Fix syntax. * Fix syntax. * List remote branches instead. * Clean up script. * Untab. * Add branch list for debugging. * Untab. * Specify remote branches. * Skip second stage if no first. * Typo * Error. * List all branches. * Fix logic. * Fix spacing. * Fix output * Fix parameters. * Fix merge to use origin. * Remove debug echos. * Add git config. * Fix PR creation. * Fix PR creation. * Fix automatic push. * Increase retry. (#1402) * Add Firestore test history report (#1403) * Add --firestore flag to report Firestore history. * Add Firestore report to nightly cron job. * Remove multiline commands. * Revert "Remove multiline commands." This reverts commit 6393ae9. * Fix tabbing. * Fix packaging test detection for Firestore. * Revert "OR Query Implementation (#1335)" (#1399) This reverts commit 11332d4. * Wrap the real-time RemoteConfig test in flaky-block to automatically retry. (#1406) * Only use flaky_test on android * Update the iOS version used by FTL (#1408) * Kick off nightly packaging an hour earlier (#1409) * Fix crash on gma::Initialize without a Firebase App (#1320) Initialize Util before using Util::FIndClass. Also add an integration test for initializing gma without a firebase app. * build: pass along the CMake path (#1410) When building the subproject, we would invoke `cmake` directly, relying on the path lookup of the tool, which may or may not match the currently executing CMake. Use `CMAKE_COMMAND` which gives us the path to the current CMake executable ensuring that we use the same CMake for building the dependencies. * Add log to gsutil fetch. (#1411) * Update mobile dependencies - Thu Aug 03 2023 (#1413) * Update mobile dependencies - Thu Aug 03 2023 ### Android - com.google.firebase.firebase_bom → 32.2.2 ### iOS - Firebase/Analytics → 10.13.0 - Firebase/AppCheck → 10.13.0 - Firebase/Auth → 10.13.0 - Firebase/Core → 10.13.0 - Firebase/CoreOnly → 10.13.0 - Firebase/Crashlytics → 10.13.0 - Firebase/Database → 10.13.0 - Firebase/DynamicLinks → 10.13.0 - Firebase/Firestore → 10.13.0 - Firebase/Functions → 10.13.0 - Firebase/Installations → 10.13.0 - Firebase/Messaging → 10.13.0 - Firebase/RemoteConfig → 10.13.0 - Firebase/Storage → 10.13.0 - Google-Mobile-Ads-SDK → 10.9.0 > Created by [Update Android and iOS dependencies workflow](https://github.com/firebase/firebase-cpp-sdk/actions/runs/5755777170). * Ensure using a Swift bridging header that supports both arm64 and x86_64. * Add release note about i386 no longer being supported. * Remove armv7 as well. --------- Co-authored-by: firebase-workflow-trigger-bot <firebase-workflow-trigger-bot@google.com> Co-authored-by: Jon Simantov <jsimantov@google.com> Co-authored-by: almostmatt@google.com <almostmatt@google.com> * feat(auth): Add emulator support (#1400) * add auth emulator support * fix ios number parse * update the documentation part * reduce lint warning * more lint warnings * code format * use environment to decide using emulator or not * fix a typo * add readme entry for FirebaseApp.GetApps() * update for review comment * add missing ` --------- Co-authored-by: Cynthia Jiang <cynthiajiang@google.com> --------- Co-authored-by: Jon Simantov <jsimantov@google.com> Co-authored-by: Tom Andersen <tom-andersen@users.noreply.github.com> Co-authored-by: Matthew Hyndman <almostmatt@google.com> Co-authored-by: a-maurice <amaurice@google.com> Co-authored-by: Saleem Abdulrasool <compnerd@compnerd.org> Co-authored-by: firebase-workflow-trigger[bot] <80733318+firebase-workflow-trigger[bot]@users.noreply.github.com> Co-authored-by: firebase-workflow-trigger-bot <firebase-workflow-trigger-bot@google.com> Co-authored-by: Cynthia J <cynthiajoan@users.noreply.github.com> Co-authored-by: Cynthia Jiang <cynthiajiang@google.com>

…1425) * iOS: Remove Analytics dependency from GMA integration test (#1386) * Try using CoreOnly instead of Analytics cocoapod for tests. * Update readme. * Add stub workflow for updating feature branches. * Add script to merge main into all active feature branches on a regular schedule. (#1394) * Add workflow for automatically updating feature branches weekly. * Corrected filename, and added inputs. * Remove trailing spaces. * Fix workflow. * Fix workflow name. * Fix syntax. * Fix syntax. * List remote branches instead. * Clean up script. * Untab. * Add branch list for debugging. * Untab. * Specify remote branches. * Skip second stage if no first. * Typo * Error. * List all branches. * Fix logic. * Fix spacing. * Fix output * Fix parameters. * Fix merge to use origin. * Remove debug echos. * Add git config. * Fix PR creation. * Fix PR creation. * Fix automatic push. * Increase retry. (#1402) * Add Firestore test history report (#1403) * Add --firestore flag to report Firestore history. * Add Firestore report to nightly cron job. * Remove multiline commands. * Revert "Remove multiline commands." This reverts commit 6393ae9. * Fix tabbing. * Fix packaging test detection for Firestore. * Revert "OR Query Implementation (#1335)" (#1399) This reverts commit 11332d4. * Wrap the real-time RemoteConfig test in flaky-block to automatically retry. (#1406) * Only use flaky_test on android * Update the iOS version used by FTL (#1408) * Kick off nightly packaging an hour earlier (#1409) * Fix crash on gma::Initialize without a Firebase App (#1320) Initialize Util before using Util::FIndClass. Also add an integration test for initializing gma without a firebase app. * build: pass along the CMake path (#1410) When building the subproject, we would invoke `cmake` directly, relying on the path lookup of the tool, which may or may not match the currently executing CMake. Use `CMAKE_COMMAND` which gives us the path to the current CMake executable ensuring that we use the same CMake for building the dependencies. * Add log to gsutil fetch. (#1411) * Update mobile dependencies - Thu Aug 03 2023 (#1413) * Update mobile dependencies - Thu Aug 03 2023 ### Android - com.google.firebase.firebase_bom → 32.2.2 ### iOS - Firebase/Analytics → 10.13.0 - Firebase/AppCheck → 10.13.0 - Firebase/Auth → 10.13.0 - Firebase/Core → 10.13.0 - Firebase/CoreOnly → 10.13.0 - Firebase/Crashlytics → 10.13.0 - Firebase/Database → 10.13.0 - Firebase/DynamicLinks → 10.13.0 - Firebase/Firestore → 10.13.0 - Firebase/Functions → 10.13.0 - Firebase/Installations → 10.13.0 - Firebase/Messaging → 10.13.0 - Firebase/RemoteConfig → 10.13.0 - Firebase/Storage → 10.13.0 - Google-Mobile-Ads-SDK → 10.9.0 > Created by [Update Android and iOS dependencies workflow](https://github.com/firebase/firebase-cpp-sdk/actions/runs/5755777170). * Ensure using a Swift bridging header that supports both arm64 and x86_64. * Add release note about i386 no longer being supported. * Remove armv7 as well. --------- Co-authored-by: firebase-workflow-trigger-bot <firebase-workflow-trigger-bot@google.com> Co-authored-by: Jon Simantov <jsimantov@google.com> Co-authored-by: almostmatt@google.com <almostmatt@google.com> * feat(auth): Add emulator support (#1400) * add auth emulator support * fix ios number parse * update the documentation part * reduce lint warning * more lint warnings * code format * use environment to decide using emulator or not * fix a typo * add readme entry for FirebaseApp.GetApps() * update for review comment * add missing ` --------- Co-authored-by: Cynthia Jiang <cynthiajiang@google.com> * Update merge-to-main script (#1420) * Before creating the merge-main PR, branch it off main, so merge conflicts can be addressed in the branch rather than by committing to main. * Format. * Update release version number for M135, 11.4.0 (#1421) * feat(auth): enable emulator support on desktop (#1423) * add auth emulator support * fix ios number parse * update the documentation part * reduce lint warning * more lint warnings * code format * use environment to decide using emulator or not * fix a typo * add readme entry for FirebaseApp.GetApps() * update for review comment * add missing ` * make json request always do restrict json * add readme info * prepare for review * some minor tweak * code format * update review comments * update the unit test after request json change --------- Co-authored-by: Cynthia Jiang <cynthiajiang@google.com> * Internal Updates. (#1422) * Add logic to trim webp parameter from native image urls --------- Co-authored-by: Jon Simantov <jsimantov@google.com> Co-authored-by: Tom Andersen <tom-andersen@users.noreply.github.com> Co-authored-by: Matthew Hyndman <almostmatt@google.com> Co-authored-by: a-maurice <amaurice@google.com> Co-authored-by: Saleem Abdulrasool <compnerd@compnerd.org> Co-authored-by: firebase-workflow-trigger[bot] <80733318+firebase-workflow-trigger[bot]@users.noreply.github.com> Co-authored-by: firebase-workflow-trigger-bot <firebase-workflow-trigger-bot@google.com> Co-authored-by: Cynthia J <cynthiajoan@users.noreply.github.com> Co-authored-by: Cynthia Jiang <cynthiajiang@google.com> Co-authored-by: nakirekommula <126601200+nakirekommula@users.noreply.github.com>

AlmostMatt added 3 commits May 11, 2023 17:16

Fix for gma fail to initialize without firebase app

10930e0

format

efcde40

release notes

8f154d4

AlmostMatt added the tests-requested: quick Trigger a quick set of integration tests. label May 11, 2023

github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels May 11, 2023

AlmostMatt changed the title ~~Fix Gma initialize~~ Fix crash on gma::Initialize without a Firebase App May 11, 2023

AlmostMatt marked this pull request as ready for review May 11, 2023 21:34

github-actions bot added the tests: failed This PR's integration tests failed. label May 11, 2023

Add the same wait to gma terminate as the other test cases

903ad74

AlmostMatt added the tests-requested: quick Trigger a quick set of integration tests. label May 11, 2023

github-actions bot added tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels May 11, 2023

AlmostMatt marked this pull request as draft May 11, 2023 23:40

firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label May 11, 2023

AlmostMatt added the tests-requested: quick Trigger a quick set of integration tests. label May 12, 2023

github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels May 12, 2023

firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label May 12, 2023

Merge branch 'main' into amatt-gma-initialize

a0acf3d

github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests: failed This PR's integration tests failed. labels May 19, 2023

firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label May 19, 2023

jonsimantov reviewed Jul 24, 2023

View reviewed changes

Merge branch 'main' into amatt-gma-initialize

c2311d1

AlmostMatt marked this pull request as ready for review August 1, 2023 19:04

AlmostMatt added the tests-requested: quick Trigger a quick set of integration tests. label Aug 1, 2023

github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Aug 1, 2023

capitalization

f3e25f0

jonsimantov reviewed Aug 1, 2023

View reviewed changes

gma/src/android/gma_android.cc Show resolved Hide resolved

jonsimantov reviewed Aug 1, 2023

View reviewed changes

gma/integration_test/src/integration_test.cc Outdated Show resolved Hide resolved

AlmostMatt added 2 commits August 1, 2023 15:18

also call util terminate if class is not found

6178f69

minor change based on feedback

4e35897

AlmostMatt added the tests-requested: quick Trigger a quick set of integration tests. label Aug 1, 2023

github-actions bot added tests: succeeded This PR's integration tests succeeded. and removed tests-requested: quick Trigger a quick set of integration tests. labels Aug 1, 2023

firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Aug 1, 2023

jonsimantov reviewed Aug 2, 2023

View reviewed changes

gma/integration_test/src/integration_test.cc Outdated Show resolved Hide resolved

jonsimantov approved these changes Aug 2, 2023

View reviewed changes

AlmostMatt added 2 commits August 2, 2023 00:05

remove error code check as waitforcompletion does it

29f371b

Merge branch 'main' into amatt-gma-initialize

13469f5

AlmostMatt enabled auto-merge (squash) August 2, 2023 04:07

AlmostMatt merged commit 18367c0 into main Aug 2, 2023

github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests: succeeded This PR's integration tests succeeded. labels Aug 2, 2023

firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Aug 2, 2023

AlmostMatt mentioned this pull request Aug 2, 2023

[Question] Segfault on firebase::gma::Initialize #1247

Closed

github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Aug 2, 2023

AlmostMatt deleted the amatt-gma-initialize branch August 15, 2023 17:51

firebase locked and limited conversation to collaborators Sep 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix crash on gma::Initialize without a Firebase App #1320

Fix crash on gma::Initialize without a Firebase App #1320

AlmostMatt commented May 11, 2023 •

edited

Loading

github-actions bot commented May 11, 2023 •

edited

Loading

jonsimantov left a comment

Fix crash on gma::Initialize without a Firebase App #1320

Fix crash on gma::Initialize without a Firebase App #1320

Conversation

AlmostMatt commented May 11, 2023 • edited Loading

Description

Testing

Type of Change

Notes

github-actions bot commented May 11, 2023 • edited Loading

Integration test with FLAKINESS (succeeded after retry)

jonsimantov left a comment

Choose a reason for hiding this comment

AlmostMatt commented May 11, 2023 •

edited

Loading

github-actions bot commented May 11, 2023 •

edited

Loading