Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glean.js makes iOS to crash for too much stack recursion #1599

Closed
bakulf opened this issue Aug 20, 2021 · 8 comments · Fixed by #1911
Closed

Glean.js makes iOS to crash for too much stack recursion #1599

bakulf opened this issue Aug 20, 2021 · 8 comments · Fixed by #1911
Assignees
Labels
bug Something isn't working p2 Medium Criticality Issues

Comments

@bakulf
Copy link
Collaborator

bakulf commented Aug 20, 2021

We should fix this issue, asap! For now, let's disable glean for iOS.

┆Issue is synchronized with this Jira Bug

@bakulf bakulf added bug Something isn't working iOS p2 Medium Criticality Issues labels Aug 20, 2021
@data-sync-user
Copy link
Collaborator

➤ Nma-Sie-Anyene Davenport commented:

No action needed from PSP team

@brizental
Copy link
Contributor

Hey folks, just an update here. Unfortunately I am not able to reproduce this bug in my own iPhone.

If possible, it would be useful to check if the crash persists even when pings are sent without passing a signal from C++ to QML. Since the crash happens exactly at the time the signal is sent, maybe there is some issue with the communication between the two layers.

Also, do we have access to the JavaScript console logs at the time of the crash? That could potentially have important information about the crash. I would suggest enabling the logPings debug feature in Glean to have the most logs Glean will provide.

@data-sync-user
Copy link
Collaborator

➤ Adrienne Davenport commented:

JR Johnson - Can you give us information regarding the frequency of this occurrence, please?

@data-sync-user
Copy link
Collaborator

➤ JR Johnson commented:

Adrienne Davenport I am not familiar with this issue currently. Can you give us identification criteria? If its as simple as inbound referring to IOS app crashes, then we have not seen any an impact ticket wise currently.

@data-sync-user
Copy link
Collaborator

➤ Sarah Bird commented:

It’s actually Beatriz Rizental Machado who’s working on this. But I’m assigning it to me as I’m helping with fix verification and any other way I can, and it would be weird to have Beatriz on our sprint board.

@data-sync-user
Copy link
Collaborator

➤ Sarah Bird commented:

Adrienne Davenport JR Johnson this is not happening in production as we disabled glean on iOS to prevent it happening in production.

@birdsarah birdsarah mentioned this issue Sep 29, 2021
13 tasks
birdsarah added a commit that referenced this issue Oct 18, 2021
Fixes #1599, #1930 

* Adds new mvpn_debug flag
* Enables glean for iOS with fix for glean crash in debug mode QT_LIBRARY_SUFFIX="" (#1599)
* replaces all QT_DEBUG checks with MVPN_DEBUG
* removes QDebug imports in src code
* use requirements.txt everywhere (#1930)
* embed glean v0.23 (this PR has actually done multiple upgrades, I'll try and keep this comment accurate)
* initializes glean only after telemetry policy window has been shown
* ensures Glean.setUploadEnabled isn't called till after glean has been initialized
* sets Glean channels to distinguish between `staging` and `production` pings
* Does not gate glean calls, lets Glean handle its own business
* Uses Glean.shutdown method on quit
* Removes custom uploader and lets glean handle retries
@data-sync-user
Copy link
Collaborator

➤ Valentina Virlics commented:

Do QA need to check something here? If yes, please provide some details or if it will be covered by 2.6 normal/regression testing. Thank you! Sarah Bird

@data-sync-user
Copy link
Collaborator

➤ Sarah Bird commented:

If iOS doesn’t crash, you’re good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2 Medium Criticality Issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants