From fef62e1f2bbf064100e6e262a10653f2374bfd2b Mon Sep 17 00:00:00 2001 From: tsepez Date: Thu, 22 Sep 2016 11:37:13 -0700 Subject: Null CPDF_CountedObj::m_pObj prior to deletion This gives additional protection in case of re-entry. Also make CFX_CountRef more robust in face of errors. BUG=649229 Review-Url: https://codereview.chromium.org/2364673002 --- core/fpdfapi/fpdf_page/cpdf_countedobject.h | 4 +++- core/fxcrt/cfx_string_data_template.h | 2 +- core/fxcrt/include/cfx_count_ref.h | 9 ++++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/core/fpdfapi/fpdf_page/cpdf_countedobject.h b/core/fpdfapi/fpdf_page/cpdf_countedobject.h index e7f4ab6af0..c61e024589 100644 --- a/core/fpdfapi/fpdf_page/cpdf_countedobject.h +++ b/core/fpdfapi/fpdf_page/cpdf_countedobject.h @@ -20,8 +20,10 @@ class CPDF_CountedObject { m_pObj = ptr; } void clear() { // Now you're all weak ptrs ... - delete m_pObj; + // Guard against accidental re-entry. + T* pObj = m_pObj; m_pObj = nullptr; + delete pObj; } T* get() const { return m_pObj; } T* AddRef() { diff --git a/core/fxcrt/cfx_string_data_template.h b/core/fxcrt/cfx_string_data_template.h index 225020a7dd..5f7bfa1f83 100644 --- a/core/fxcrt/cfx_string_data_template.h +++ b/core/fxcrt/cfx_string_data_template.h @@ -85,7 +85,7 @@ class CFX_StringDataTemplate { // Since the count increments with each new pointer, the largest value is // the number of pointers that can fit into the address space. The size of // the address space itself is a good upper bound on it. - intptr_t m_nRefs; // Would prefer ssize_t, but no windows support. + intptr_t m_nRefs; // |FX_STRSIZE| is currently typedef'd as |int|. // TODO(palmer): It should be a |size_t|, or at least unsigned. diff --git a/core/fxcrt/include/cfx_count_ref.h b/core/fxcrt/include/cfx_count_ref.h index d709efb67c..a879967b28 100644 --- a/core/fxcrt/include/cfx_count_ref.h +++ b/core/fxcrt/include/cfx_count_ref.h @@ -56,15 +56,22 @@ class CFX_CountRef { CountedObj(Args... params) : ObjClass(params...), m_RefCount(0) {} CountedObj(const CountedObj& src) : ObjClass(src), m_RefCount(0) {} + ~CountedObj() { m_RefCount = 0; } bool HasOneRef() const { return m_RefCount == 1; } void Retain() { m_RefCount++; } void Release() { - if (--m_RefCount <= 0) + ASSERT(m_RefCount); + if (--m_RefCount == 0) delete this; } private: + // To ensure ref counts do not overflow, consider the worst possible case: + // the entire address space contains nothing but pointers to this object. + // Since the count increments with each new pointer, the largest value is + // the number of pointers that can fit into the address space. The size of + // the address space itself is a good upper bound on it. intptr_t m_RefCount; }; -- cgit v1.2.3