Inspector destructors unconditionally delete objects

RESOLVED FIXED

Status

RESOLVED FIXED
17 years ago
12 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

17 years ago
+	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) {}
(Assignee)

Comment 1

17 years ago
+	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) {}
Status: NEW → ASSIGNED
Summary: inCSSValueSearch::~inCSSValueSearch() unconditionally calls delete mResult → Inspector destructors unconditionally delete objects
fix is to init that member variables to nsnull in the constructor, I guess?
(Assignee)

Comment 3

17 years ago
Created attachment 106709 [details] [diff] [review]
patch
(Assignee)

Updated

17 years ago
Attachment #106709 - Flags: superreview?(bzbarsky)
Attachment #106709 - Flags: review?(caillon)
you are inconsistent with nsnull vs. 0 here
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()
Attachment #106709 - Flags: review?(caillon) → review+
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.
Attachment #106709 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 7

16 years ago
fixed Nov 21 07:55  
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Product: Core → Other Applications
QA Contact: timeless → dom-inspector
You need to log in before you can comment on or make changes to this bug.