Skip to content

Commit 5a7a1db

Browse files
Tom TaylorAndroid (Google) Code Review
Tom Taylor
authored and
Android (Google) Code Review
committed
Merge "Fix deadlock in PDU code"
2 parents 1d60133 + 733e697 commit 5a7a1db

File tree

1 file changed

+91
-86
lines changed

1 file changed

+91
-86
lines changed

‎core/java/com/google/android/mms/pdu/PduPersister.java‎

Lines changed: 91 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -519,98 +519,99 @@ private void loadAddress(long msgId, PduHeaders headers) {
519519
* @throws MmsException Failed to load some fields of a PDU.
520520
*/
521521
public GenericPdu load(Uri uri) throws MmsException {
522-
PduCacheEntry cacheEntry;
523-
synchronized(PDU_CACHE_INSTANCE) {
524-
if (PDU_CACHE_INSTANCE.isUpdating(uri)) {
525-
if (LOCAL_LOGV) {
526-
Log.v(TAG, "load: " + uri + " blocked by isUpdating()");
527-
}
528-
try {
529-
PDU_CACHE_INSTANCE.wait();
530-
} catch (InterruptedException e) {
531-
Log.e(TAG, "load: ", e);
532-
}
533-
cacheEntry = PDU_CACHE_INSTANCE.get(uri);
534-
if (cacheEntry != null) {
535-
return cacheEntry.getPdu();
522+
GenericPdu pdu = null;
523+
PduCacheEntry cacheEntry = null;
524+
int msgBox = 0;
525+
long threadId = -1;
526+
try {
527+
synchronized(PDU_CACHE_INSTANCE) {
528+
if (PDU_CACHE_INSTANCE.isUpdating(uri)) {
529+
if (LOCAL_LOGV) {
530+
Log.v(TAG, "load: " + uri + " blocked by isUpdating()");
531+
}
532+
try {
533+
PDU_CACHE_INSTANCE.wait();
534+
} catch (InterruptedException e) {
535+
Log.e(TAG, "load: ", e);
536+
}
537+
cacheEntry = PDU_CACHE_INSTANCE.get(uri);
538+
if (cacheEntry != null) {
539+
return cacheEntry.getPdu();
540+
}
536541
}
542+
// Tell the cache to indicate to other callers that this item
543+
// is currently being updated.
544+
PDU_CACHE_INSTANCE.setUpdating(uri, true);
537545
}
538-
// Tell the cache to indicate to other callers that this item
539-
// is currently being updated.
540-
PDU_CACHE_INSTANCE.setUpdating(uri, true);
541-
}
542546

543-
Cursor c = SqliteWrapper.query(mContext, mContentResolver, uri,
544-
PDU_PROJECTION, null, null, null);
545-
PduHeaders headers = new PduHeaders();
546-
Set<Entry<Integer, Integer>> set;
547-
long msgId = ContentUris.parseId(uri);
548-
int msgBox;
549-
long threadId;
547+
Cursor c = SqliteWrapper.query(mContext, mContentResolver, uri,
548+
PDU_PROJECTION, null, null, null);
549+
PduHeaders headers = new PduHeaders();
550+
Set<Entry<Integer, Integer>> set;
551+
long msgId = ContentUris.parseId(uri);
550552

551-
try {
552-
if ((c == null) || (c.getCount() != 1) || !c.moveToFirst()) {
553-
throw new MmsException("Bad uri: " + uri);
554-
}
553+
try {
554+
if ((c == null) || (c.getCount() != 1) || !c.moveToFirst()) {
555+
throw new MmsException("Bad uri: " + uri);
556+
}
555557

556-
msgBox = c.getInt(PDU_COLUMN_MESSAGE_BOX);
557-
threadId = c.getLong(PDU_COLUMN_THREAD_ID);
558+
msgBox = c.getInt(PDU_COLUMN_MESSAGE_BOX);
559+
threadId = c.getLong(PDU_COLUMN_THREAD_ID);
558560

559-
set = ENCODED_STRING_COLUMN_INDEX_MAP.entrySet();
560-
for (Entry<Integer, Integer> e : set) {
561-
setEncodedStringValueToHeaders(
562-
c, e.getValue(), headers, e.getKey());
563-
}
561+
set = ENCODED_STRING_COLUMN_INDEX_MAP.entrySet();
562+
for (Entry<Integer, Integer> e : set) {
563+
setEncodedStringValueToHeaders(
564+
c, e.getValue(), headers, e.getKey());
565+
}
564566

565-
set = TEXT_STRING_COLUMN_INDEX_MAP.entrySet();
566-
for (Entry<Integer, Integer> e : set) {
567-
setTextStringToHeaders(
568-
c, e.getValue(), headers, e.getKey());
569-
}
567+
set = TEXT_STRING_COLUMN_INDEX_MAP.entrySet();
568+
for (Entry<Integer, Integer> e : set) {
569+
setTextStringToHeaders(
570+
c, e.getValue(), headers, e.getKey());
571+
}
570572

571-
set = OCTET_COLUMN_INDEX_MAP.entrySet();
572-
for (Entry<Integer, Integer> e : set) {
573-
setOctetToHeaders(
574-
c, e.getValue(), headers, e.getKey());
575-
}
573+
set = OCTET_COLUMN_INDEX_MAP.entrySet();
574+
for (Entry<Integer, Integer> e : set) {
575+
setOctetToHeaders(
576+
c, e.getValue(), headers, e.getKey());
577+
}
576578

577-
set = LONG_COLUMN_INDEX_MAP.entrySet();
578-
for (Entry<Integer, Integer> e : set) {
579-
setLongToHeaders(
580-
c, e.getValue(), headers, e.getKey());
581-
}
582-
} finally {
583-
if (c != null) {
584-
c.close();
579+
set = LONG_COLUMN_INDEX_MAP.entrySet();
580+
for (Entry<Integer, Integer> e : set) {
581+
setLongToHeaders(
582+
c, e.getValue(), headers, e.getKey());
583+
}
584+
} finally {
585+
if (c != null) {
586+
c.close();
587+
}
585588
}
586-
}
587-
588-
// Check whether 'msgId' has been assigned a valid value.
589-
if (msgId == -1L) {
590-
throw new MmsException("Error! ID of the message: -1.");
591-
}
592589

593-
// Load address information of the MM.
594-
loadAddress(msgId, headers);
595-
596-
int msgType = headers.getOctet(PduHeaders.MESSAGE_TYPE);
597-
PduBody body = new PduBody();
590+
// Check whether 'msgId' has been assigned a valid value.
591+
if (msgId == -1L) {
592+
throw new MmsException("Error! ID of the message: -1.");
593+
}
598594

599-
// For PDU which type is M_retrieve.conf or Send.req, we should
600-
// load multiparts and put them into the body of the PDU.
601-
if ((msgType == PduHeaders.MESSAGE_TYPE_RETRIEVE_CONF)
602-
|| (msgType == PduHeaders.MESSAGE_TYPE_SEND_REQ)) {
603-
PduPart[] parts = loadParts(msgId);
604-
if (parts != null) {
605-
int partsNum = parts.length;
606-
for (int i = 0; i < partsNum; i++) {
607-
body.addPart(parts[i]);
595+
// Load address information of the MM.
596+
loadAddress(msgId, headers);
597+
598+
int msgType = headers.getOctet(PduHeaders.MESSAGE_TYPE);
599+
PduBody body = new PduBody();
600+
601+
// For PDU which type is M_retrieve.conf or Send.req, we should
602+
// load multiparts and put them into the body of the PDU.
603+
if ((msgType == PduHeaders.MESSAGE_TYPE_RETRIEVE_CONF)
604+
|| (msgType == PduHeaders.MESSAGE_TYPE_SEND_REQ)) {
605+
PduPart[] parts = loadParts(msgId);
606+
if (parts != null) {
607+
int partsNum = parts.length;
608+
for (int i = 0; i < partsNum; i++) {
609+
body.addPart(parts[i]);
610+
}
608611
}
609612
}
610-
}
611613

612-
GenericPdu pdu = null;
613-
switch (msgType) {
614+
switch (msgType) {
614615
case PduHeaders.MESSAGE_TYPE_NOTIFICATION_IND:
615616
pdu = new NotificationInd(headers);
616617
break;
@@ -657,16 +658,20 @@ public GenericPdu load(Uri uri) throws MmsException {
657658
default:
658659
throw new MmsException(
659660
"Unrecognized PDU type: " + Integer.toHexString(msgType));
661+
}
662+
} finally {
663+
synchronized(PDU_CACHE_INSTANCE) {
664+
if (pdu != null) {
665+
assert(PDU_CACHE_INSTANCE.get(uri) == null);
666+
// Update the cache entry with the real info
667+
cacheEntry = new PduCacheEntry(pdu, msgBox, threadId);
668+
PDU_CACHE_INSTANCE.put(uri, cacheEntry);
669+
}
670+
PDU_CACHE_INSTANCE.setUpdating(uri, false);
671+
PDU_CACHE_INSTANCE.notifyAll(); // tell anybody waiting on this entry to go ahead
672+
}
660673
}
661-
662-
synchronized(PDU_CACHE_INSTANCE ) {
663-
assert(PDU_CACHE_INSTANCE.get(uri) == null);
664-
// Update the cache entry with the real info
665-
cacheEntry = new PduCacheEntry(pdu, msgBox, threadId);
666-
PDU_CACHE_INSTANCE.put(uri, cacheEntry);
667-
PDU_CACHE_INSTANCE.notifyAll(); // tell anybody waiting on this entry to go ahead
668-
return pdu;
669-
}
674+
return pdu;
670675
}
671676

672677
private void persistAddress(

0 commit comments

Comments
 (0)