Leaking mEnv_HandlePool

VERIFIED FIXED in Future

Status

MailNews Core
Database
P2
normal
VERIFIED FIXED
19 years ago
10 years ago

People

(Reporter: Patrick C. Beard, Assigned: Bienvenu)

Tracking

({memory-leak})

Trunk
Future
memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

19 years ago
Leak detector indicates that morkEnv::mEnv_HandlePool is leaking. The URL above 
shows the point of allocation that leaks.
Keywords: mlk
(Assignee)

Comment 1

19 years ago
these are mine now, I guess.
Assignee: davidmc → bienvenu

Comment 2

19 years ago
Marking M16 (not M15 stopper)
Target Milestone: --- → M16
(Assignee)

Comment 3

19 years ago
accepting
Status: NEW → ASSIGNED

Comment 4

18 years ago
Not M16 stopper.  Marking M17.
Target Milestone: M16 → M17

Comment 5

18 years ago
moving to future.
Target Milestone: M17 → Future

Comment 6

18 years ago
Can we afford for this to remain futured?  If nothing else, apart from the 
mail/news usage, mork does get used in global history, impacting the rest of the 
product.
(Assignee)

Updated

18 years ago
Keywords: mail1
(Assignee)

Comment 7

18 years ago
changing priorities
Priority: P3 → P2

Comment 8

18 years ago
David,

How big of a leak is this? Will the whole db or folder get leaked because of this?
(Assignee)

Comment 9

18 years ago
it's a very small leak.

Comment 10

18 years ago
marking nsbeta1- due to the fact that it's a small leak.
Keywords: nsbeta1-
I'll let bienvenu do the review on this, since it is mork.  (I'll do the sr=)

the patch seems ok to me.  hwaara, how did you test it?

Comment 13

18 years ago
I compiled with the change and then ran mailnews for a good while. I didn't
really know how to test, but I deleted and moved lots of mails...

bienvenu?
(Assignee)

Comment 14

18 years ago
No, I'm not sure this is right. mEnvHandles come from a pool, and are returned
to the pool when CloseEnv is called. So just out and out deleting them is not
the way this was designed to work, as far as I can tell.

Comment 15

18 years ago
So why is this bug filed in the first place if it ain't leaking?

If it is leaking, then do you have any suggestions on approaches to fix this?
(Assignee)

Comment 16

18 years ago
I meant that your fix was wrong, not that there wasn't a leak. It is, however, a
tiny, tiny, leak. We have bigger fish to fry.

For one thing, CloseEnv sets mEnv_HandlePool to null, so your fix will not do
anything - mEnv_HandlePool will always be null by the time the environment
destructor gets called.

If I knew what the correct fix was, I would fix it. This is *not* code that I
wrote; the person who wrote this code is long gone. So it requires
investigation. Sorry I can't be more helpful.

Comment 17

18 years ago
Bienvenu, isn't it as easy as replacing the set-to-null code with a nullcheck
and then delete?
(Assignee)

Comment 18

18 years ago
no, we pool the handles. This means we allocate them from a pool, and return
them to a pool when done. At least, that's the idea.

Comment 19

18 years ago
then I guess this bug will have to wait some more for a fix :(

Updated

17 years ago
Blocks: 92580
QA Contact: lchiang → stephend

Updated

17 years ago
No longer blocks: 92580
Summary: [MLK] Leaking mEnv_HandlePool → Leaking mEnv_HandlePool
(Assignee)

Comment 20

17 years ago
plussing; will be fixed when mork changes land.
Keywords: nsbeta1+
(Assignee)

Comment 21

17 years ago
should be fixed now.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
After David's big checkin, I no longer see this when launching mail, reading an 
IMAP message, loading a folder or deleting messages.

Verified FIXED, using Purify.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.