If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Inspector destructors unconditionally delete objects

RESOLVED FIXED

Status

Other Applications
DOM Inspector
RESOLVED FIXED
15 years ago
10 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

2.40 KB, patch
Christopher Aillon (sabbatical, not receiving bugmail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

15 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

15 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

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

Updated

15 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

15 years ago
fixed Nov 21 07:55  
Status: ASSIGNED → RESOLVED
Last Resolved: 15 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.