1262393 - Race condition between main UI thread and Gecko thread for displaying/updating tabs

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: General, defect)

Description

[chhsiao]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160315153207

Steps to reproduce:

This race condition used the same setting as in Bug 1262387.

I'm currently developing a race detector for unordered messages on Android systems.
The execution I tested had the following steps on an Nexus 4:

1. Visited developers.android.com
2. Scrolled down and pressed "Material Design for Developers"
3. Scrolled down then pressed back.
4. Repeat the above steps a couple times.

This race is found by my race detector so in the actual execution
there was no erroneous behavior but it is a race between the main UI thread and the Gecko thread so theoretically it could manifest.



Actual results:

Various types of messages that updated tab attributes were posted to Gecko thread,
while the main UI thread read those attributes to display the tab. According to Android's API Guides (http://developer.android.com/guide/components/processes-and-threads.html), these updates should be done in the main UI thread.

Example:

When a document is loaded, a "Content:StateChange" message would be posted to Gecko thread, which would set the corresponding tab into reader mode (Tabs.java:525),
then post an asynchronous UI update message to the UI thread (Tabs.java:526).
The change in the reader mode should only be seen after this asynchronous update. However, before processing this asynchronous update, the UI thread might process other pending messages by calling the OnTabsChangedListeners (Tabs.java:670), one of which is BrowserToolbar.onTabChanged(), which would in turn sees that the tab is already in reader mode.

The following lines contain the stack traces of two accesses in this race:

Write in Gecko thread: opcode=+iput-quick addr=1115013712 value=0
[1] at org.mozilla.gecko.Tab.setState(Tab.java:385)
[2] at org.mozilla.gecko.Tab.handleDocumentStop(Tab.java:723)
[3] at org.mozilla.gecko.Tabs.handleMessage(Tabs.java:525)
[4] at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:325)
[5] at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:248)
[6] at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2315)
[7] at org.mozilla.gecko.GeckoThread.run(GeckoThread.java)

Read in UI thread: opcode=+iget-quick addr=1115013712 value=0
[1] at org.mozilla.gecko.Tab.isEnteringReaderMode(Tab.java:617)
[2] at org.mozilla.gecko.toolbar.ToolbarDisplayLayout.updateFromTab(ToolbarDisplayLayout.java:291)
[3] at org.mozilla.gecko.toolbar.BrowserToolbar.onTabChanged(BrowserToolbar.java:500)
[4] at org.mozilla.gecko.Tabs$5.run(Tabs.java:670)
[5] at android.os.Handler.handleCallback(Handler.java:741)
[6] at android.os.Handler.dispatchMessage(Handler.java:96)
[7] at android.os.Looper.loop(Looper.java:150)
[8] at android.app.ActivityThread.main(ActivityThread.java:5237)
[9] at java.lang.reflect.Method.invoke(Method.java)
[10] at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:746)

Similar problems happen in most of the messages processed in Tabs.handleMessage(). Here is another example:

Read in UI thread: opcode=+iget-object-quick addr=1115013628 value=96227
[1] at org.mozilla.gecko.Tab.loadFavicon(Tab.java:427)
[2] at org.mozilla.gecko.BrowserApp.onTabChanged(BrowserApp.java:326)
[3] at org.mozilla.gecko.Tabs$5.run(Tabs.java:670)
[4] at android.os.Handler.handleCallback(Handler.java:741)
[5] at android.os.Handler.dispatchMessage(Handler.java:96)
[6] at android.os.Looper.loop(Looper.java:150)
[7] at android.app.ActivityThread.main(ActivityThread.java:5237)
[8] at java.lang.reflect.Method.invoke(Method.java)
[9] at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:746)

Write in Gecko thread: opcode=+iput-object-quick addr=1115013628 value=0
[1] at org.mozilla.gecko.Tab.clearFavicon(Tab.java:493)
[2] at org.mozilla.gecko.Tab.handleLocationChange(Tab.java:677)
[3] at org.mozilla.gecko.Tabs.handleMessage(Tabs.java:510)
[4] at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:325)
[5] at org.mozilla.gecko.EventDispatcher.dispatchEvent(EventDispatcher.java:248)
[6] at org.mozilla.gecko.GeckoAppShell.handleGeckoMessage(GeckoAppShell.java:2315)
[7] at org.mozilla.gecko.GeckoThread.run(GeckoThread.java)


Expected results:

Updates to tab attributes should be done in the UI threads, maybe through the OnTabChangedListeners.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

What version of Firefox for Android are you testing with? If you did not use Nightly would you please retest with it? https://nightly.mozilla.org

Comment 2

[chhsiao]

Sure I'll do it and update the info recently. Thanks for the reply.

Comment 3

[Stefan Deiac]

Due to the lack of additional info, I'll close this issue as WFM. If the reporter or someone else think to reopen the issue feel free to do it, thanks.