[MLK] proxy events need to copy string arguments

VERIFIED FIXED in M13

Status

SeaMonkey
General
P3
normal
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: Gagan, Assigned: dougt)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
To handle passing strings correctly across using proxy objects, we need to make
a copy of them in the proxy events. When this gets fixed, pls assign this back
to me so that I can remove the extra delete that is there in webshell (with my
checkins for status code messages #10333)

Updated

19 years ago
Summary: proxy events need to copy string arguments → [MLK] proxy events need to copy string arguments

Comment 1

19 years ago
Adding [MLK] in this bug's summary line to mark this as a memory leak since its

showing up in Boehm.
(Assignee)

Comment 2

19 years ago
who is passing strings using an async proxy?
(Assignee)

Comment 3

19 years ago
gagan, is this a show stopper for you?

Updated

19 years ago
Target Milestone: M12

Comment 4

19 years ago
need to fix by beta.  (10-25-99 meeting)

Updated

19 years ago
Blocks: 17432
(Assignee)

Updated

19 years ago
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 5

19 years ago
it would be safe and quicker for you to change your interface than me coping
strings.
(Reporter)

Comment 6

19 years ago
nope its not a show stopper but I think this should still be deferred not
wontfix. Fix it someday damnit! :)
(Assignee)

Updated

19 years ago
Status: RESOLVED → REOPENED
(Assignee)

Updated

19 years ago
Status: REOPENED → RESOLVED
Last Resolved: 19 years ago19 years ago
Resolution: WONTFIX → LATER
(Assignee)

Comment 7

19 years ago
for you gagan.

Comment 8

19 years ago
Hey Doug, you promised me before I left on vacation that you'd fix this SOON!
:^)

Updated

19 years ago
Status: RESOLVED → REOPENED

Comment 9

19 years ago
I really think this needs to be fixed. It's problematic to have one piece of
code allocate a string and put it into an event, and have another piece of code
running asynchronously delete the string. It's inconsistent, leads to bugs, and
is a maintenance nightmare. Please fix.
(Assignee)

Comment 10

19 years ago
coping strings is a HUGE time waste.  I suggested changing your parameters so
that they can be addrefed, not to have some out of other piece of code that
cleans up "almost leaked" strings.

In necko's case, every async notification to the UI would have to do an strlen
and an allocation.  This would be a significate time hit on your critical
threads.  I would must rather see a string interface that would do this for
you.

Updated

19 years ago
Resolution: LATER → ---

Comment 11

19 years ago
Back in ancient history, someone decided that refcounting strings was bad.
Copying is the price we pay today. Revisiting this is not possible now.

Given this decision, the proxy code should copy the string when constructing the
event, and delete the string when deleting the event. That's the only way to
deal with this consistently (until I can re-raise the nsIString issue. ;-)
(Assignee)

Updated

19 years ago
Target Milestone: M12 → M13
(Assignee)

Comment 12

19 years ago
I still disagree, but will argue later.  Moving this to m13.

Updated

19 years ago
Blocks: 20666
(Assignee)

Comment 13

19 years ago
*** Bug 20666 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

19 years ago
Status: REOPENED → ASSIGNED
(Assignee)

Comment 14

19 years ago
fix in hand.
(Assignee)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

19 years ago
fix checked in.

will now copy strings (string | wstring) defined in an interface.

Comment 16

19 years ago
Adding verifyme keyword.
Keywords: verifyme

Comment 17

19 years ago
Basedon last eng comments, and not reopen of thie bug. I am marking Verified.
Status: RESOLVED → VERIFIED

Updated

18 years ago
No longer blocks: 17432
Product: Browser → Seamonkey

Updated

10 years ago
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.