Open Bug 449240 Opened 11 years ago Updated 2 years ago

|make check|: 30 tests report the same 6 leaked objects (plus others)

Categories

(Testing :: XPCShell Harness, defect)

x86
Windows 2000
defect
Not set

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: sgautherie, Assigned: peterv)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Keywords: memory-leak, Whiteboard: [See comment 4])

Attachments

(2 files, 3 obsolete files)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1a2pre) Gecko/2008080319 Minefield/3.1a2pre] (home, optim default) (W2Ksp4)

Checking for bug 448980 comment 7, I noticed 3 tests which have another leak too.

test_bug299716.js
test_bug396129.js
test_bug428341.js

{{
   2 BackstagePass                                  20       20        1        1 (    1,00 +/-     0,00)       61        2 (   18,53 +/-     9,38)
  37 XPCNativeScriptableShared                     108      108      273        1 (   10,05 +/-     3,11)        0        0 (    0,00 +/-     0,00)
  40 XPCWrappedNative                               48       48      971        1 (  471,99 +/-   274,67)     3256        1 (  510,80 +/-   273,03)
  41 XPCWrappedNativeProto                          28       28      120        1 (   58,07 +/-    33,55)        0        0 (    0,00 +/-     0,00)
 213 nsStringBuffer                                  8     3504     4898      438 ( 1057,47 +/-   411,34)     7572      438 ( 1111,59 +/-   462,71)
 223 nsSystemPrincipal                              32       32        1        1 (    1,00 +/-     0,00)       15        1 (    4,38 +/-     1,88)
}}
NB: test_bug299716.js has an 439th nsSB leaked.
Flags: wanted1.9.1?
Oh, just noticed that these tests are all in <xpcshell-simple/test_extensionmanager/unit/>.
Component: Testing → Add-ons Manager
Product: Core → Toolkit
QA Contact: testing → extension.manager
Blocks: mlkTests
(In reply to comment #1)
> Oh, just noticed that these tests are all in
> <xpcshell-simple/test_extensionmanager/unit/>.

cc'ing Mossop - this is code he spent a lot of time working on.
I'm already seeing this. But at the moment I have neither the time nor the know-how to figure out what might be going on here
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080911182230 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

Disregard the 3 bugs mentioned in comment 0, which don't apply anymore.

We have (now) 30 tests which fail with various leaks,
but seem to have at least this common one, seen in
test_placesTxn.js and test_urlformatter.js:
[
1 BackstagePass
1 XPCNativeScriptableShared
1 XPCWrappedNative
1 XPCWrappedNativeProto
1 nsStringBuffer
1 nsSystemPrincipal
]
NB: The many more nsStringBuffer previously reported were fixed by bug 453146.

test_0030_general.js and test_0110_general.js
have the same report with only 1 XPCWrappedNative and 1 XPCWrappedNativeProto more.

The 26 others tests have much longer leak reports.
Depends on: 453146
No longer depends on: 448980
Summary: |make check|: 3 tests reports the same 6 leaked objects → |make check|: 30 tests report the same 6 leaked objects (plus others)
Whiteboard: [See comment 4]
I filed Bug 454964 for the two App Update tests mentioned in comment #4
(In reply to comment #4)
> We have (now) 30 tests which fail with various leaks,
> but seem to have at least this common one, seen in
> test_placesTxn.js and test_urlformatter.js:

Unfortunately that doesn't mean they have the same source. I'll attach patches for test_placesTxn.js (leak caused by the test) and test_urlformatter.js (leak caused by the code being tested). Do you have a list of the other tests that leak?
Comment on attachment 338630 [details] [diff] [review]
Patch for test_placesTxn and test_urlformatter

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080915194703 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

I confirm that this patch fixes these 2 tests.
(In reply to comment #6)
> Unfortunately that doesn't mean they have the same source.

Unfortunately, only the report(s) seems to be the "same" :-(
Component: Add-ons Manager → General
Depends on: 454964
Product: Toolkit → Core
QA Contact: extension.manager → general
Component: General → Testing
QA Contact: general → testing
> Do you have a list of the other tests that leak?

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080915194703 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

Now, I have 1 more, total 31:
*2 are fixed by your patch for this bug.
*2 "are" bug 454964.
*27 others, to fix in this bug or in separate ones.
Comment on attachment 338630 [details] [diff] [review]
Patch for test_placesTxn and test_urlformatter


Peter, wanna request review ?
Assignee: nobody → peterv
Attachment #338630 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 339687 [details] [diff] [review]
Patch for test_placesTxn, test_urlformatter and autocomplete
[See bug 464114]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080925220718 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

I confirm that this patch fixes all the (23) Autocomplete and Places tests :-)
(only) The 4+2 others remain.

Peter, wanna request review ?
Peter, ping ?
IIRC, peter said he'll get back to this bug (later).
Depends on: 464114
Comment on attachment 339687 [details] [diff] [review]
Patch for test_placesTxn, test_urlformatter and autocomplete
[See bug 464114]

'autocomplete' part moved to bug 464114, at last.
Attachment #339687 - Attachment is obsolete: true
Comment on attachment 338630 [details] [diff] [review]
Patch for test_placesTxn and test_urlformatter

Peter, wanna request review ?
Attachment #338630 - Attachment is obsolete: false
Attachment #339687 - Attachment description: Patch for test_placesTxn, test_urlformatter and autocomplete → Patch for test_placesTxn, test_urlformatter and autocomplete [See bug 464114]
Depends on: 456414
After bug 464114 which fixed 25 tests;
with a few "new" tests.
Attachment #338776 - Attachment is obsolete: true
test_statement_wrapper_automatically.js is probably bug 464202 and bug 457743.
Depends on: 464909
(In reply to comment #19)
> test_statement_wrapper_automatically.js is probably bug 464202 and bug 457743.

Confirmed: bug 457743 fixed the 2 following tests/leaks:
{
_tests\xpcshell-simple\test_dm\unit
test_removeDownloadsByTimeframe.js

_tests\xpcshell-simple\test_storage\unit
test_statement_wrapper_automatically.js
}
Attachment #348114 - Attachment is obsolete: true
Comment on attachment 338630 [details] [diff] [review]
Patch for test_placesTxn and test_urlformatter

NB: The other part of test_placesTxn.js leak was fixed. (See bug 464816 comment 7.)
Depends on: 468761
-> TUnit, although ideally bugs on tests should go in the component that the test is testing. This bug mentions a bunch of different tests though, so I'm just sticking it in testing.
Component: Testing → TUnit
Flags: wanted1.9.1?
Product: Core → Testing
QA Contact: testing → tunit
Version: Trunk → unspecified
I just noticed that, compared to
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081217 Minefield/3.2a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/214c605bfb64)


[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081217 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/4a4a42520901
 +http://hg.mozilla.org/comm-central/rev/877154dd96a2 + bug 469606 patch)

{
_tests\xpcshell-simple\test_toolkit_contentprefs\unit
test_bug248970.js
test_contentPrefs.js
}
are not included to be run.

{
_tests\xpcshell-simple\test_necko\unit
test_bug248970_cache.js
}
runs without leaking.
Though, it reports "96 entries" vs "183 entries" in FF, so I wonder.
Version: unspecified → Trunk
Serge: I'd prefer if you filed separate bugs on individual tests that leak. Should be easier to get traction on them that way.
Depends on: 470235
Depends on: 470239
Depends on: 470241
Depends on: 470244
(In reply to comment #25)
> Serge: I'd prefer if you filed separate bugs on individual tests that leak.

Done.
I agree that the remaining cases are likely to be unrelated one to another.
Depends on: 469523
No longer depends on: 456414
You need to log in before you can comment on or make changes to this bug.