This repository has been archived by the owner. It is now read-only.

potential data races in NewChatActivity.java #410

Open
yulin2 opened this Issue Feb 13, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@yulin2

yulin2 commented Feb 13, 2014

Dear developers of ChatSecureAndroid,

I'm a Ph.D. student and I'm doing research on checking data race for Android apps. I found there are data races that may lead to potential bugs in NewChatActivity.java:

The AsyncTask in "startGroupChat" method writes to field "mRequestedChatId" at line 2173. This task is eventually be called by "onLoadFinished" method at line 300 through method "resolveIntent". However, "onLoadFinished" also uses "mRequestedChatId" at lines 301-303. This lead to data race on "mRequestedChatId". Do you think this race may lead to potential bugs? Do you need synchronization on "mRequestedChatId"?

"mRequestedChatId" is also written by "startChat" method at line 1944 and 1950, this can also be a race with the written at line 2173.

Also, the AsyncTask invokes "showChat" method and "showChat" access the GUI elements "mChatPagerAdapter" and "mChatPager" at line 690 and 700. Android documents say we shouldn't access GUI outside main thread. So this should be a problem. It also leads to a race on these GUI widgets and the Cursor at line 690, since "mChatPagerAdapter", "mChatPager" and the Cursor is also read/written by main thread.
A fix could be put the statements at 690 and 700 into a Handler or runOnUiThread method.

Thanks,
Yu

@yulin2

This comment has been minimized.

Show comment
Hide comment
@yulin2

yulin2 Mar 4, 2014

Dear developers,

I reported this issue two weeks ago. Do you have any comments on these races? Will they lead to concurrency bugs? It'd be better if we can eliminate these races.

Thanks,
Yu

yulin2 commented Mar 4, 2014

Dear developers,

I reported this issue two weeks ago. Do you have any comments on these races? Will they lead to concurrency bugs? It'd be better if we can eliminate these races.

Thanks,
Yu

@n8fr8

This comment has been minimized.

Show comment
Hide comment
@n8fr8

n8fr8 Mar 6, 2014

Member

We are reworking NewChatActivity right now, and will take all of this important feedback into consideration.

Member

n8fr8 commented Mar 6, 2014

We are reworking NewChatActivity right now, and will take all of this important feedback into consideration.

@n8fr8 n8fr8 added this to the v14.2 milestone May 8, 2015

@n8fr8 n8fr8 added BUG SHOULD labels May 8, 2015

@n8fr8 n8fr8 modified the milestones: v14.2, v15 AWESOME APP Jul 8, 2015

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.