Report possible misuse of ConcurrentHashMap
Created by: baigd
Hi, Developers of smc/Indic-Keyboard,
I am writing to report two race issues on use of ConcurrentHashMap. The issues are reported by our tool in an automatic way. Although manually confirmed, they could be false positives, given we do not know the specification of the program. We would very appreciate if you could check below for details and confirm with us whether they are real problems. For more information, please refer to our website: http://sav.sutd.edu.sg/?page_id=2845
File: java/src/com/android/inputmethod/latin/utils/ExecutorUtils.java Location: Line (50,74) Description: "line 50 is not guarded with any lock so that its thread-safety is dedicated to the ConcurrentHashMap implementation. On the other hand, sExecutorMap is synchronized at line 68, assuming perhaps that it would provide exclusive access to sExecutorMap and therefore the remove method is invoked without other threads modifying the object. Unfortunately, this misunderstanding of the API specification leads to a data race. When the two methods execute concurrently, a bug may manifest in the following interleaving order. Suppose Thread1 executes method getExecutor and Thread2 executes method shutdownAllExecutors. Thread1 is scheduled to execute first and it invokes the get method at line 6, which returns a nonnull executor. At the same time, Thread2 is scheduled to execute and it removes all the executors in the shared field sExecutorMap. As a result, the getExecutor returns a non-null executor while all executors are actually shutdown and removed from the hash map. "
File: java/src/org.smc.inputmethod/indic/personalization/PersonalizationHelper.java Location: Line (44, 88) Description: "The ""sLangUser HistoryDictCache"" field is an instance of ConcurrentHashMap. There are two methods, i.e., ""getUserHistoryDictionary"" and ""runGCOnAll DictionariesIfRequired"", accessing this field. However, in method ""runGCOnAllDictionariesIfRequired"", there is no explicit synchronization on ""sLangUserHistoryDictCache"" when it is updated (line 88). As a result, there is a potential data race. Suppose Thread1 is created to execute method ""getUserHistoryDictionary()"" and Thread2 is created to execute method ""RunCOnAllOpenedUserHistoryDictionaries()"". Thread1 is scheduled to execute first and it checks that the ConcurrentHashMap contains the key localStr (line 43), then Thread2 is scheduled to run and it invokes the ""runGCOnAllDictionariesIfRequired ()"" with the shared variable ""sLangUserHistoryDict-Cache"". Since the code run by Thread2 does not synchronize on the ""sLangUserHistoryDictCache()"" object, it can access (in this case remove) an entry from the object. Suppose the entry is exactly the one indicated by ""localeStr"". When Thread1 resumes and runs to line 44, a null value is returned which violates the program logic and may results in NullPointerException to be thrown later."
--- Want to back this issue? **Post a bounty on it!** We accept bounties via Bountysource.