180127 - Inspector destructors unconditionally delete objects

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[timeless]

+	mResults	0xcdcdcdcd

inCSSValueSearch::~inCSSValueSearch() line 67 + 26 bytes
inCSSValueSearch::`scalar deleting destructor'(unsigned int 1) + 15 bytes
inCSSValueSearch::Release(inCSSValueSearch * const 0x02f962d0) line 70 + 179 bytes

steps: 
Create this object and delete it without ever using it.
js> for (a in C) try { o=C[a].createInstance(); for (i in I) o instanceof I[i] }
catch (e) {}

Comment 1

[timeless]

+	mBits	0xcdcdcdcd ""

operator delete(void * 0xcdcdcdcd) line 7 + 9 bytes
inBitmap::~inBitmap() line 54 + 18 bytes
inBitmap::`scalar deleting destructor'(unsigned int 1) + 15 bytes
inBitmap::Release(inBitmap * const 0x03076750) line 57 + 179 bytes

const C=Components.classes,I=Components.interfaces;
var o;for (a in C) if (!/dom|box/i.test(a)) try {o=C[a].createInstance(); for (j
in I) o instanceof I[j]} catch (e) {}

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

fix is to init that member variables to nsnull in the constructor, I guess?

Comment 3

[timeless]

Attachment: patch

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

you are inconsistent with nsnull vs. 0 here

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 106709 [details] [diff] [review]
patch

> inBitmap::inBitmap()
>+  : mBits(0) 

nsnull here as biesi already pointed out that this is a PRUint8* member (you
promised to turn the pointer into an int in another bug, please file?).


> inCSSValueSearch::inCSSValueSearch()
>+  : mIsActive(PR_FALSE),
>+    mResultCount(0),
>+    mHoldResults(PR_TRUE),
>+    mResults(nsnull),
>+    mLastResult(nsnull),

mBaseURL 

>+    mTextCriteria(nsnull),

This would give you a compiler warning about members being init'd in the wrong
order.	Please fix that.

>+    mReturnRelativeURLs(PR_FALSE),
>+    mNormalizeChromeURLs(PR_FALSE),
>+    mPropertyCount(0)
> {
>   NS_INIT_ISUPPORTS();
>-  
>-  mHoldResults = PR_TRUE;
>-  mReturnRelativeURLs = PR_FALSE;
>-  mNormalizeChromeURLs = PR_FALSE;
>-  mResultCount = 0;
> 
>   mProperties = new nsCSSProperty[100];

Eek.  The above line scares me.  We certainly have over 100 properties (CSS2
has 120 alone)..... File a bug about looking at this?

>-  mPropertyCount = 0;
>   mCSSUtils = do_GetService(kInspectorCSSUtilsCID);
> }
> 


> inFileSearch::inFileSearch()
>+  : mIsActive(PR_FALSE),
>+    mResultCount(0),

init mHoldResults

>+    mBasePath(nsnull),
>+    mReturnRelativePaths(PR_FALSE),
>+    mTextCriteria(nsnull),
>+    mFilenameCriteria(nsnull),
>+    mFilenameCriteriaCount(0),
>+    mSearchRecursive(PR_FALSE),
>+    mDirsSearched(0),
>+    mSearchLoop(nsnull)
> {
>   NS_INIT_ISUPPORTS();
>-
>-  mSearchLoop = 0;
> }
> 
> inFileSearch::~inFileSearch()

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 106709 [details] [diff] [review]
patch

sr=bzbarsky with caillon's issues fixed and if you check to make sure there are
no other reordering warnings lurking.

Comment 7

[timeless]

fixed Nov 21 07:55