336914 - mAdoptFreeCount not maintained correctly

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

I decided to track down this output from startup + shutdown:

 => mAdoptCount:           1507
 => mAdoptFreeCount:       1478  --  LEAKED 29 !!!

As far as I can tell, this happens because mAdoptFreeCount is incorrect.  Specifically, calling

  nsTAdoptingString_CharT::operator=( const self_type& str )

will increment mAdoptCount by 1 but not change mAdoptFreeCount, which is wrong since the buffer just got passed from one string to another.  Unfortunately, all the logging code is in a different file (nsSubString.cpp, to be exact).  So I can't just add a logging statement here.

Perhaps we should simply move all of nsTString.cpp (which is just the constructor in question) into nsTSubString.cpp?

Comment 1

[Darin Fisher]

Good catch.  You could just avoid calling Adopt and set the member fields directly.  The important thing to do is to properly finalize the existing string who's members are being replaced with the adopted value.

Comment 2

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I found this by just adding ctor/dtor logging for the buffer when we adopt/adoptfree and then looking at the adopt stacks not matched by free....  Not sure whether we want to add that to the tree permanently; let me know.

Comment 3

[Darin Fisher]

Yeah, even if only as an optional #ifdef, I think that'd be great.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: This seems to work

Comment 5

[Darin Fisher]

Comment on attachment 221117 [details] [diff] [review]
This seems to work

s/refcound/refcount/

Comment 6

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 7

[[e-mail]]

This patch, in combination with the static string detector, blows up on Windows; nsLocalFileWin::Load calls IsFile calls ResolveAndStat which does lots of string manipulation, unfortunately while the static string detector is active.

Note that the static string detector does otherwise work in that it found a host of static strings in nsWindowsHooks[Utils].cpp which I recently fixed.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't really know anything about the static string detector.  How does this patch affect it?