From 8c3c51bb33dff53168827f338d6c6c50c9825c08 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Thu, 24 Jun 2010 21:49:02 -0700
Subject: Fix a bug in sp<> and wp<> which could cause memory corruptions

when assigning a smart pointer to another one, we need to make
sure to read all the data we need from the right-hand-side
reference (the assignee) before we decRef the assigned.

This bug would cause linked-list of smart-pointers to fail
miserably.

Change-Id: Ibb554c15fddf909f7737c632b7c80322e80ea93f
---
 include/utils/RefBase.h | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

(limited to 'include/utils/RefBase.h')

diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h
index bd7f28cae3..9c64ac0443 100644
--- a/include/utils/RefBase.h
+++ b/include/utils/RefBase.h
@@ -333,9 +333,10 @@ sp<T>::~sp()
 
 template<typename T>
 sp<T>& sp<T>::operator = (const sp<T>& other) {
-    if (other.m_ptr) other.m_ptr->incStrong(this);
+    T* otherPtr(other.m_ptr);
+    if (otherPtr) otherPtr->incStrong(this);
     if (m_ptr) m_ptr->decStrong(this);
-    m_ptr = other.m_ptr;
+    m_ptr = otherPtr;
     return *this;
 }
 
@@ -351,9 +352,10 @@ sp<T>& sp<T>::operator = (T* other)
 template<typename T> template<typename U>
 sp<T>& sp<T>::operator = (const sp<U>& other)
 {
-    if (other.m_ptr) other.m_ptr->incStrong(this);
+    U* otherPtr(other.m_ptr);
+    if (otherPtr) otherPtr->incStrong(this);
     if (m_ptr) m_ptr->decStrong(this);
-    m_ptr = other.m_ptr;
+    m_ptr = otherPtr;
     return *this;
 }
 
@@ -466,10 +468,12 @@ wp<T>& wp<T>::operator = (T* other)
 template<typename T>
 wp<T>& wp<T>::operator = (const wp<T>& other)
 {
-    if (other.m_ptr) other.m_refs->incWeak(this);
+    weakref_type* otherRefs(other.m_refs);
+    T* otherPtr(other.m_ptr);
+    if (otherPtr) otherRefs->incWeak(this);
     if (m_ptr) m_refs->decWeak(this);
-    m_ptr = other.m_ptr;
-    m_refs = other.m_refs;
+    m_ptr = otherPtr;
+    m_refs = otherRefs;
     return *this;
 }
 
@@ -478,8 +482,9 @@ wp<T>& wp<T>::operator = (const sp<T>& other)
 {
     weakref_type* newRefs =
         other != NULL ? other->createWeak(this) : 0;
+    T* otherPtr(other.m_ptr);
     if (m_ptr) m_refs->decWeak(this);
-    m_ptr = other.get();
+    m_ptr = otherPtr;
     m_refs = newRefs;
     return *this;
 }
@@ -498,10 +503,12 @@ wp<T>& wp<T>::operator = (U* other)
 template<typename T> template<typename U>
 wp<T>& wp<T>::operator = (const wp<U>& other)
 {
-    if (other.m_ptr) other.m_refs->incWeak(this);
+    weakref_type* otherRefs(other.m_refs);
+    U* otherPtr(other.m_ptr);
+    if (otherPtr) otherRefs->incWeak(this);
     if (m_ptr) m_refs->decWeak(this);
-    m_ptr = other.m_ptr;
-    m_refs = other.m_refs;
+    m_ptr = otherPtr;
+    m_refs = otherRefs;
     return *this;
 }
 
@@ -510,8 +517,9 @@ wp<T>& wp<T>::operator = (const sp<U>& other)
 {
     weakref_type* newRefs =
         other != NULL ? other->createWeak(this) : 0;
+    U* otherPtr(other.m_ptr);
     if (m_ptr) m_refs->decWeak(this);
-    m_ptr = other.get();
+    m_ptr = otherPtr;
     m_refs = newRefs;
     return *this;
 }
-- 
cgit v1.2.3-59-g8ed1b