possible memory leak ? nsIPrivateTextRange leak in IMETextTxn::CollapseTextSelection

RESOLVED FIXED

Status

()

Core
Internationalization
RESOLVED FIXED
15 years ago
11 years ago

People

(Reporter: Frank Tang, Assigned: smaug)

Tracking

({intl, mlk})

Trunk
x86
Windows XP
intl, mlk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
in IMETextTxn.cpp IMETextTxn::CollapseTextSelection we have a loop
nsIPrivateTextRange *textRange;
...
mRangeList->GetLength(&textRangeListLength);
...
for(i=0;i<textRangeListLength;i++)
{
 mRangeList->Item(i,&textRange);
 ... 
 textRange->GetRangeType(&textRangeType);
 ...
 textRange->GetRangeStart(&selectionStart);
 ...
 textRange->GetRangeEnd(&selectionEnd);
 ..
}

The mRangeList->Item will addref before it return textRange, but it seems that
we fogot to call textRange->Release() before the for loop.

Roy- can  you double check that code. It seems we should add a 
textRange->Release() there. But you need to run through debuger to see
the refcount to make sure that is the right thing to do. 
 ...


This code will be called when we call input method.
Or you could use an nsCOMPtr and avoid these sorts of problems....
(Reporter)

Comment 2

15 years ago
Created attachment 103731 [details] [diff] [review]
patch

I try this on window. It does not cause any double free
yokoyama, can you step through this code
nhotta, can you try this patch on mac and run ime 
make sure it does not cause any problem
katakai san- can you do the samething on linux?

Updated

15 years ago
Keywords: mlk

Comment 3

15 years ago
code issue? QA to yokoyama@netscape.com for now.
Keywords: intl
QA Contact: ruixu → yokoyama
Created attachment 250351 [details] [diff] [review]
proposed patch

mRangeList->Item() AddRefs; http://lxr.mozilla.org/seamonkey/source/content/events/src/nsPrivateTextRange.cpp#121
So using nsCOMPtr and getter_AddRefs
Assignee: tetsuroy → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #250351 - Flags: superreview?(bzbarsky)
Attachment #250351 - Flags: review?(bzbarsky)
Attachment #250351 - Attachment is patch: true
Attachment #250351 - Attachment mime type: text/x-patch → text/plain
Comment on attachment 250351 [details] [diff] [review]
proposed patch

Looks ok, though I'm technically not an editor peer...
Attachment #250351 - Flags: superreview?(bzbarsky)
Attachment #250351 - Flags: superreview+
Attachment #250351 - Flags: review?(bzbarsky)
Attachment #250351 - Flags: review+
(In reply to comment #5)
> Looks ok, though I'm technically not an editor peer...
> 
I know, but since you had commented here (only few years ago),
I thought you want to r+ :)

Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.