From d7784628368404ff53caf080640cd9ff32775fd3 Mon Sep 17 00:00:00 2001 From: David Sobreira Marques Date: Sun, 17 Apr 2011 12:46:50 -0300 Subject: Fixing concurrency issue on IccPhoneBookInterfaceManager. All the reads/updates methods are synchronous calls that rely on an unique lock object in order to wait for the asynchronous simcard operations to complete and return appropriate results Concurent calls to these methods will cause errors when one completed operation will unlock all waiting calls generating inconsistent results on some of the method calls. Change-Id: I8b87004ac039bcb971b8369f7640281f1bf9eb35 Signed-off-by: David Sobreira Marques --- .../telephony/IccPhoneBookInterfaceManager.java | 55 +++++++++++++--------- .../cdma/RuimPhoneBookInterfaceManager.java | 11 ++--- .../gsm/SimPhoneBookInterfaceManager.java | 11 ++--- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/telephony/java/com/android/internal/telephony/IccPhoneBookInterfaceManager.java b/telephony/java/com/android/internal/telephony/IccPhoneBookInterfaceManager.java index 2f22d7448ed5..45562ca96e44 100644 --- a/telephony/java/com/android/internal/telephony/IccPhoneBookInterfaceManager.java +++ b/telephony/java/com/android/internal/telephony/IccPhoneBookInterfaceManager.java @@ -24,6 +24,7 @@ import android.os.Message; import android.os.ServiceManager; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; /** * SimPhoneBookInterfaceManager to provide an inter-process communication to @@ -63,14 +64,14 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { " total " + recordSize[1] + " #record " + recordSize[2]); } - mLock.notifyAll(); + notifyPending(ar); } break; case EVENT_UPDATE_DONE: ar = (AsyncResult) msg.obj; synchronized (mLock) { success = (ar.exception == null); - mLock.notifyAll(); + notifyPending(ar); } break; case EVENT_LOAD_DONE: @@ -84,11 +85,20 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { records.clear(); } } - mLock.notifyAll(); + notifyPending(ar); } break; } } + + private void notifyPending(AsyncResult ar) { + if (ar.userObj == null) { + return; + } + AtomicBoolean status = (AtomicBoolean) ar.userObj; + status.set(true); + mLock.notifyAll(); + } }; public IccPhoneBookInterfaceManager(PhoneBase phone) { @@ -150,15 +160,12 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { synchronized(mLock) { checkThread(); success = false; - Message response = mBaseHandler.obtainMessage(EVENT_UPDATE_DONE); + AtomicBoolean status = new AtomicBoolean(false); + Message response = mBaseHandler.obtainMessage(EVENT_UPDATE_DONE, status); AdnRecord oldAdn = new AdnRecord(oldTag, oldPhoneNumber); AdnRecord newAdn = new AdnRecord(newTag, newPhoneNumber); adnCache.updateAdnBySearch(efid, oldAdn, newAdn, pin2, response); - try { - mLock.wait(); - } catch (InterruptedException e) { - logd("interrupted while trying to update by search"); - } + waitForResult(status); } return success; } @@ -197,14 +204,11 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { synchronized(mLock) { checkThread(); success = false; - Message response = mBaseHandler.obtainMessage(EVENT_UPDATE_DONE); + AtomicBoolean status = new AtomicBoolean(false); + Message response = mBaseHandler.obtainMessage(EVENT_UPDATE_DONE, status); AdnRecord newAdn = new AdnRecord(newTag, newPhoneNumber); adnCache.updateAdnByIndex(efid, newAdn, index, pin2, response); - try { - mLock.wait(); - } catch (InterruptedException e) { - logd("interrupted while trying to update by index"); - } + waitForResult(status); } return success; } @@ -243,15 +247,12 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { synchronized(mLock) { checkThread(); - Message response = mBaseHandler.obtainMessage(EVENT_LOAD_DONE); + AtomicBoolean status = new AtomicBoolean(false); + Message response = mBaseHandler.obtainMessage(EVENT_LOAD_DONE, status); adnCache.requestLoadAllAdnLike(efid, adnCache.extensionEfForEf(efid), response); - try { - mLock.wait(); - } catch (InterruptedException e) { - logd("interrupted while trying to load from the SIM"); - } + waitForResult(status); } - return records; + return records; } protected void checkThread() { @@ -265,6 +266,16 @@ public abstract class IccPhoneBookInterfaceManager extends IIccPhoneBook.Stub { } } + protected void waitForResult(AtomicBoolean status) { + while (!status.get()) { + try { + mLock.wait(); + } catch (InterruptedException e) { + logd("interrupted while trying to update by search"); + } + } + } + private int updateEfForIccType(int efid) { // Check if we are trying to read ADN records if (efid == IccConstants.EF_ADN) { diff --git a/telephony/java/com/android/internal/telephony/cdma/RuimPhoneBookInterfaceManager.java b/telephony/java/com/android/internal/telephony/cdma/RuimPhoneBookInterfaceManager.java index 6e12f24a2409..dd1efdfbbec4 100644 --- a/telephony/java/com/android/internal/telephony/cdma/RuimPhoneBookInterfaceManager.java +++ b/telephony/java/com/android/internal/telephony/cdma/RuimPhoneBookInterfaceManager.java @@ -16,6 +16,8 @@ package com.android.internal.telephony.cdma; +import java.util.concurrent.atomic.AtomicBoolean; + import android.os.Message; import android.util.Log; @@ -56,14 +58,11 @@ public class RuimPhoneBookInterfaceManager extends IccPhoneBookInterfaceManager recordSize = new int[3]; //Using mBaseHandler, no difference in EVENT_GET_SIZE_DONE handling - Message response = mBaseHandler.obtainMessage(EVENT_GET_SIZE_DONE); + AtomicBoolean status = new AtomicBoolean(false); + Message response = mBaseHandler.obtainMessage(EVENT_GET_SIZE_DONE, status); phone.getIccFileHandler().getEFLinearRecordSize(efid, response); - try { - mLock.wait(); - } catch (InterruptedException e) { - logd("interrupted while trying to load from the RUIM"); - } + waitForResult(status); } return recordSize; diff --git a/telephony/java/com/android/internal/telephony/gsm/SimPhoneBookInterfaceManager.java b/telephony/java/com/android/internal/telephony/gsm/SimPhoneBookInterfaceManager.java index feb508ab10e7..001e1bde5406 100644 --- a/telephony/java/com/android/internal/telephony/gsm/SimPhoneBookInterfaceManager.java +++ b/telephony/java/com/android/internal/telephony/gsm/SimPhoneBookInterfaceManager.java @@ -16,6 +16,8 @@ package com.android.internal.telephony.gsm; +import java.util.concurrent.atomic.AtomicBoolean; + import android.os.Message; import android.util.Log; @@ -56,14 +58,11 @@ public class SimPhoneBookInterfaceManager extends IccPhoneBookInterfaceManager { recordSize = new int[3]; //Using mBaseHandler, no difference in EVENT_GET_SIZE_DONE handling - Message response = mBaseHandler.obtainMessage(EVENT_GET_SIZE_DONE); + AtomicBoolean status = new AtomicBoolean(false); + Message response = mBaseHandler.obtainMessage(EVENT_GET_SIZE_DONE, status); phone.getIccFileHandler().getEFLinearRecordSize(efid, response); - try { - mLock.wait(); - } catch (InterruptedException e) { - logd("interrupted while trying to load from the SIM"); - } + waitForResult(status); } return recordSize; -- cgit v1.2.3-59-g8ed1b