504504 - xpcshell-tests: test_downloadOffline.js leaks intermittently

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090715 SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/d190d9b6ccd1
 +http://hg.mozilla.org/comm-central/rev/773809454cf2)

{
TEST-PASS | e:\Dvlp\Mozilla\Hg\objdir-SeaMonkey_\mozilla\_tests\xpcshell\test_imap\unit\test_downloadOffline.js | test passed

== BloatView: ALL (cumulative) LEAK STATISTICS

     |<----------------Class--------------->|<-----Bytes------>|<----------------Objects
                                              Per-Inst   Leaked    Total
   0 TOTAL                                          30     2280    12272
  36 nsAStreamCopier                                64       64        5
 162 nsPipe                                        152      152        6
 173 nsRecyclingAllocatorImpl                       36       36        1
 184 nsSocketTransport                             224      224        2
 185 nsSocketTransportService                     1680     1680        1
 194 nsStringBuffer                                  8        8     4576
 204 nsThread                                       68       68        6
 207 nsTimerImpl                                    48       48        8

nsTraceRefcntImpl::DumpStatistics: 235 entries
}

Comment 1

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090530 SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/e44d9c0f4805
 +http://hg.mozilla.org/comm-central/rev/62f2c362a9a4 + bug 493008 patches)

Ftr, this build already had this bug.

Comment 2

[Zane U. Ji]

Attachment: Don't end the test in a listenser

Comment 3

[David :Bienvenu]

Comment on attachment 423142 [details] [diff] [review]
Don't end the test in a listenser

thx, Zane.

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 423142 [details] [diff] [review]
Don't end the test in a listenser

While there,

>@@ -97,19 +97,21 @@ var UrlListener =

85     if (!gDownloadedOnce) {
86       gDownloadedOnce = true;
87       gIMAPInbox.downloadAllForOffline(UrlListener, null);
88       return;
89     }
90     else {

Could you remove this "_else_ after return"?

>       }
>-      endTest();
>     }
>     do_timeout(1000, endTest);

Not knowing this test, I wonder why this is needed.
Is it a case of the first of the two "endTest" calls to run wins?
Could that be documented?

>+  },
>+  finalize: function() {
>+    do_timeout(0, endTest);
>   }
> };

Comment 5

[Mark Banner (:standard8)]

I've checked this in:

http://hg.mozilla.org/comm-central/rev/376bc5f318dc

Please deal with follow-ups in a separate bug.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)

I filed bug 544396.

Comment 7

[Mark Banner (:standard8)]

Don't care about this for 3.0.x - its a test-only change and hence not a real leak, most dev work should be going on on trunk and that's where we'll be looking for leaks. Setting to "unaffected" as this wouldn't affect the 3.0 build.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)

I just have to opposite p-o-v:
test-only means zero risk, and this fixes random-orange on local and tinderboxes for SM and probably TB :-|
Your "unaffected" should be "wontfix" then :-/

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> (In reply to comment #7)
> 
> I just have to opposite p-o-v:
> test-only means zero risk, and this fixes random-orange on local and
> tinderboxes for SM and probably TB :-|

Leaks in xpcshell-tests aren't random-oranges. If they are then we haven't communicated something correctly. The Thunderbird boxes have never gone orange for leaks, and we won't look at that until FF does it (and I'm fairly sure the FF tests don't).

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)

I agree, but it will after bug 469523 and it already shows up in the logs...

Comment 11

[Mark Banner (:standard8)]

(In reply to comment #10)
> (In reply to comment #9)
> 
> I agree, but it will after bug 469523 and it already shows up in the logs...

Ok, so that hasn't been announced and I hadn't been aware of it. I'd also argue that it needs to be configurable on at least a per-repository and per-branch basis as I doubt we're going to have everything ready at the same time, and depending on the fixes required for some of the mailnews leaks we may not want to take them on the c-1.9.1 branch (not saying we won't, just saying that I have no idea).

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)

Wow! How come a trivial suggestion to land this fix on c-1.9.1 becomes a debate on what is (not) happening in core bug 469523?
It would take (me) a few minutes to land it there, and ... *$%@&!