735137 - Strict mode violation in TabsTray.onCreate

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Richard Newman [:rnewman]]

As with Bug 731116, on some devices it's not safe to use AccountManager from the main thread.

Comment 1

[Richard Newman [:rnewman]]

D/StrictMode(24292): android.os.StrictMode$LogStackTrace
D/StrictMode(24292):        at android.os.StrictMode.readAndHandleBinderCallViolations(StrictMode.java:1059)
D/StrictMode(24292):        at android.os.Parcel.readExceptionCode(Parcel.java:1304)
D/StrictMode(24292):        at android.os.Parcel.readException(Parcel.java:1273)
D/StrictMode(24292):        at android.accounts.IAccountManager$Stub$Proxy.getAccounts(IAccountManager.java:522)
D/StrictMode(24292):        at android.accounts.AccountManager.getAccountsByType(AccountManager.java:436)
D/StrictMode(24292):        at org.mozilla.gecko.TabsTray.onCreate(TabsTray.java:131)
D/StrictMode(24292):        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1047)
D/StrictMode(24292):        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:1628)
D/StrictMode(24292):        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:1680)
D/StrictMode(24292):        at android.app.ActivityThread.access$1500(ActivityThread.java:117)
D/StrictMode(24292):        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:931)
D/StrictMode(24292):        at android.os.Handler.dispatchMessage(Handler.java:99)
D/StrictMode(24292):        at android.os.Looper.loop(Looper.java:130)
D/StrictMode(24292):        at android.app.ActivityThread.main(ActivityThread.java:3703)
D/StrictMode(24292):        at java.lang.reflect.Method.invokeNative(Native Method)
D/StrictMode(24292):        at java.lang.reflect.Method.invoke(Method.java:507)
D/StrictMode(24292):        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:841)
D/StrictMode(24292):        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:599)
D/StrictMode(24292):        at dalvik.system.NativeStart.main(Native Method)

Comment 2

[Tony Chung [:tchung]]

Attachment: logcat

I'm seeing this log generated on my phone.  the StrictMode is thrown as soon as i opened the tabbrowsing dropdown menu.    I have sync enabled.

HTC Sensation, 03-12-2012 nightly build.


03-12 21:35:07.722: DEBUG/PhoneWindow(28313): couldn't save which view has focus because the focused view org.mozilla.gecko.gfx.LayerView@40565308 has no id.
03-12 21:35:07.722: INFO/GeckoApp(28313): pause
03-12 21:35:07.832: DEBUG/dalvikvm(28313): GC_EXTERNAL_ALLOC freed 505K, 58% free 4189K/9799K, external 386K/851K, paused 30ms
03-12 21:35:07.852: DEBUG/StrictMode(28313): DiskReadViolation: timestamp uptime= 254640434 ms
03-12 21:35:07.982: WARN/ResourceType(237): Skipping entry 0x7f040000 in package table 0 because it is not complex!
03-12 21:35:07.982: DEBUG/StrictMode(28313): StrictMode policy violation; ~duration=132 ms; uptime= 254640434 ms: android.os.StrictMode$StrictModeDiskReadViolation: policy=279 violation=2
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.StrictMode$AndroidBlockGuardPolicy.onReadFromDisk(StrictMode.java:873)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.database.sqlite.SQLiteDatabase.rawQueryWithFactory(SQLiteDatabase.java:1427)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.database.sqlite.SQLiteDatabase.queryWithFactory(SQLiteDatabase.java:1317)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1271)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.database.sqlite.SQLiteDatabase.query(SQLiteDatabase.java:1353)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.accounts.AccountManagerService.getAccountsByType(AccountManagerService.java:373)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.accounts.AccountManagerService.getAccounts(AccountManagerService.java:1268)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.accounts.IAccountManager$Stub.onTransact(IAccountManager.java:94)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.Binder.execTransact(Binder.java:320)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.server.SystemServer.init1(Native Method)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.server.SystemServer.main(SystemServer.java:1019)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at java.lang.reflect.Method.invokeNative(Native Method)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at java.lang.reflect.Method.invoke(Method.java:507)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:849)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:607)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at dalvik.system.NativeStart.main(Native Method)
03-12 21:35:07.982: DEBUG/StrictMode(28313): # via Binder call with stack:
03-12 21:35:07.982: DEBUG/StrictMode(28313): android.os.StrictMode$LogStackTrace
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.StrictMode.readAndHandleBinderCallViolations(StrictMode.java:1298)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.Parcel.readExceptionCode(Parcel.java:1304)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.Parcel.readException(Parcel.java:1273)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.accounts.IAccountManager$Stub$Proxy.getAccounts(IAccountManager.java:475)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.accounts.AccountManager.getAccountsByType(AccountManager.java:391)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at org.mozilla.gecko.TabsTray.onCreate(TabsTray.java:131)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1072)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:1794)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:1851)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.ActivityThread.access$1500(ActivityThread.java:132)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1038)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.Handler.dispatchMessage(Handler.java:99)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.os.Looper.loop(Looper.java:150)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at android.app.ActivityThread.main(ActivityThread.java:4293)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at java.lang.reflect.Method.invokeNative(Native Method)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at java.lang.reflect.Method.invoke(Method.java:507)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:849)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:607)
03-12 21:35:07.982: DEBUG/StrictMode(28313):     at dalvik.system.NativeStart.main(Native Method)
03-12 21:35:08.163: INFO/ActivityManager(237): Displayed org.mozilla.fennec/org.mozilla.gecko.TabsTray: +433ms

Comment 3

[Richard Newman [:rnewman]]

Give this a shot, tchung:

http://people.mozilla.com/~rnewman/strict.apk

Comment 4

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Depends on 

https://github.com/mozilla-services/android-sync/commit/1c7276a8b6364bf2f9f303b40ace16e7b05aeef0

which is still awaiting review. I figured it would be best to just lift the whole "does an account exist?" logic into Sync itself.

Comment 5

[Richard Newman [:rnewman]]

I would particularly appreciate feedback from those more familiar with AsyncTask as to whether a reference to the task needs to be kept. I presume not (it'll get pushed into a queue, then handed to a thread…), but best to be sure.

tchung, verification of the fix would be appreciated, too.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 605300 [details] [diff] [review]
Proposed patch. v1

The few examples I have seen, and the docs I have read, do not mention requiring a reference to the AsyncTask. I have seen some info stating that holding a reference is one way to "leak" memory if you're not careful.

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

I noticed the account check happening in AboutHomeContent too, but it's in a background thread there:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AboutHomeContent.java#272
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AboutHomeContent.java#316

Comment 8

[Richard Newman [:rnewman]]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> I noticed the account check happening in AboutHomeContent too, but it's in a
> background thread there:

Yup. Using this call instead (or, rather, SyncAccounts.syncAccountsExist(c), given that it's already on a background thread) would neaten that code up, too. But that's a follow-up :D

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d5da2fa866eb

Comment 10

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 605300 [details] [diff] [review]
Proposed patch. v1

I don't think we need to pass "context" as an argument to the AsyncTask. The context used by "TabsAccessor.areClientAvailable()" is the final Context declared above.

Comment 11

[Richard Newman [:rnewman]]

(In reply to Sriram Ramasubramanian [:sriram] from comment #10)

> I don't think we need to pass "context" as an argument to the AsyncTask. The
> context used by "TabsAccessor.areClientAvailable()" is the final Context
> declared above.

AccountsExistTask requires a context. It's declared here as a final local so that it's the same one used by TabsAccessor.

Comment 12

[Tony Chung [:tchung]]

Attachment: logcat from strict.apk

still reproducing on strict.apk, after setting up sync, opening 2+ tabs, and opening the tabbed browser menu.

Comment 13

[Richard Newman [:rnewman]]

tchung reported happiness with the Tinderbox build, so this should be fixed when it merges to m-c.

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d5da2fa866eb

Comment 15

[Tony Chung [:tchung]]

Verified fix on 3-16 nightly.

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4adb8d45aff4