Closed Bug 1403787 Opened 2 years ago Closed 2 years ago
Crash in java
.io .File Not Found Exception: /data/user/0/org .mozilla .firefox _beta/files/fxa .account .json (No such file or directory) at java .io .File Input Stream .open(Native Method)
59 bytes, text/x-review-board-request
3.43 KB, patch
|Details | Diff | Splinter Review|
1.42 KB, patch
|Details | Diff | Splinter Review|
This bug was filed from the Socorro interface and is report bp-ed7ab80b-bf3e-4b38-b9be-9a6290170927. ============================================================= This is another version of Bug 1400742.
Java Stack Trace java.io.FileNotFoundException: /data/user/0/org.mozilla.firefox_beta/files/fxa.account.json (No such file or directory) at java.io.FileInputStream.open(Native Method) at java.io.FileInputStream.<init>(FileInputStream.java:146) at android.app.ContextImpl.openFileInput(ContextImpl.java:505) at android.content.ContextWrapper.openFileInput(ContextWrapper.java:193) at org.mozilla.gecko.sync.Utils.readFile(Utils.java:487) at org.mozilla.gecko.fxa.authenticator.AccountPickler.unpickleParams(AccountPickler.java:166) at org.mozilla.gecko.fxa.authenticator.AndroidFxAccount.getAccountUID(AndroidFxAccount.java:217) at org.mozilla.gecko.fxa.authenticator.AndroidFxAccount.getSyncPrefs(AndroidFxAccount.java:450) at org.mozilla.gecko.fxa.devices.FxAccountDeviceRegistrator.getClientName(FxAccountDeviceRegistrator.java:295) at org.mozilla.gecko.fxa.devices.FxAccountDeviceRegistrator.makeFxADeviceCommonBuilder(FxAccountDeviceRegistrator.java:273) at org.mozilla.gecko.fxa.devices.FxAccountDeviceRegistrator.buildFxAccountDevice(FxAccountDeviceRegistrator.java:257) at org.mozilla.gecko.fxa.devices.FxAccountDeviceRegistrator.handlePushSubscriptionResponse(FxAccountDeviceRegistrator.java:174) at org.mozilla.gecko.fxa.devices.FxAccountDeviceRegistrator.handleMessage(FxAccountDeviceRegistrator.java:149) at org.mozilla.gecko.EventDispatcher$2.run(EventDispatcher.java:337) at android.os.Handler.handleCallback(Handler.java:751) at android.os.Handler.dispatchMessage(Handler.java:95) at android.os.Looper.loop(Looper.java:154) at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:43)
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
Priority: -- → P1
This is another case of pickling racing against users of the pickled file. We're failing in the process of registering current device with FxA. Device registration is kicked off whenever we reach a "token state" - Married or Engaged. That is, whenever we can actually interact with FxA. Given that we're failing because we're failing to find a pickled file, it's probably safe to assume we've never pickled before - that is, this is the first time we've reached the token state for the current account, and haven't tried accessing sync prefs either. As a short-term workaround, it should be safe to just register with FxA using a default name, as that's what we'll end up doing anyway since we won't have a custom device name set yet.
Comment on attachment 8913423 [details] Bug 1403787 - Change try/catch to catch the correct exception https://reviewboard.mozilla.org/r/184752/#review189932 Rubber stamp
Attachment #8913423 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/ef87d74f001b Register FxA device with a default client name if we fail to obtain one r=eoger
Approval Request Comment [Feature/Bug causing the regression]: Bug 1368147 [User impact if declined]: Possible crash (caused by a race condition) while Fennec is interacting with Firefox Accounts. Happens close to startup. [Is this code covered by automated tests?]: Not this in particular. [Has the fix been verified in Nightly?]: Currently on Nightly. Crash stats for 58 not yet visible in crash-stats. :( [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: Bug 1404124 is closely related. [Is the change risky?]: No. [Why is the change risky/not risky?]: If the code in question looses a race, it simply reverts to using a default client name value. [String changes made/needed]: No.
Attachment #8913782 - Flags: approval-mozilla-beta?
Comment on attachment 8913782 [details] [diff] [review] register-fxa-device-default-name.patch Fix a crash, taking it. Should be in 57b6
Attachment #8913782 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
The crash is still there in beta 57 (see ): - 57.0b5: 43 crashes - 57.0b7: 38 - 57.0b9: 36 - 57.0b11: 7  http://bit.ly/2leTOYT
We're catching the wrong exception type. FileNotFound is being re-thrown as an IllegalStateException (which, in some sense, isn't wrong). Our try/catch, however, is trying to be quite specific.
Comment on attachment 8913423 [details] Bug 1403787 - Change try/catch to catch the correct exception https://reviewboard.mozilla.org/r/184752/#review199572 Ugh, sure.
Attachment #8913423 - Flags: review?(nalexander) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/5ccd0ffe9bc7 Change try/catch to catch the correct exception r=eoger,nalexander
Approval Request Comment [Feature/Bug causing the regression]: Bug 1403787 and friends. [User impact if declined]: Possible, mainly one-time crash at the start of a first sync (caused by a race, so won't happen consistently). [Is this code covered by automated tests?]: Not this bug in particular, but the surrounding codebase. [Has the fix been verified in Nightly?]: Just landed now. [Needs manual test from QE? If yes, steps to reproduce]: Not necessary, and I expect it's pretty hard to reproduce. On a fresh install, login into FxA and start syncing. Hitting this race is going to be pretty hard (and likely will vary between devices), but crashes do show up in the crash-logs, so it's possible with enough perseverance. [List of other uplifts needed for the feature/fix]: - [Is the change risky?]: No. [Why is the change risky/not risky?]: Very trivial patch, fixes a bug (the right exception wasn't being caught) in the previous patch which addressed the crash. [String changes made/needed]: -
Comment on attachment 8928655 [details] [diff] [review] try-catch-beta.patch Let's take this in Beta and see how it goes. Beta58+.
Attachment #8928655 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
No crash in b3. Not enough volume for 57.
Comment on attachment 8928655 [details] [diff] [review] try-catch-beta.patch FYI, we didn't have a single crash on release for this bug.
Attachment #8928655 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.