Closed Bug 486489 Opened 16 years ago Closed 15 years ago

mochitest-*: intermittent leak of 6-8 kB including nsHttpConnectionInfo, nsUrlClassifier*

Categories

(Toolkit :: Safe Browsing, defect)

Product:
Toolkit
Component:
Safe Browsing
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.6a1

People

(Reporter: sgautherie, Unassigned)

References

Details

(Keywords: memory-leak, Whiteboard: [Fixed by bug 497884 and bug 496335])

Attachments

(2 files)

turn on tests [checked in]
1.21 KB, patch
ted
: review+
Details | Diff | Splinter Review
additional logging [Backout: Comment 71]
4.37 KB, patch
ted
: review+
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #472677 +++ { http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1238660210.1238670125.576.gz Linux mozilla-1.9.1 unit test on 2009/04/02 01:16:50 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 8395 bytes during test execution } The leak details look rather similar to those I see in bug 472677.
Whiteboard: [orange]
(In reply to comment #0) That one was { TinderboxPrint: mochitest-chrome<br/>1110/0/30&nbsp;<em class="testfail">LEAK</em> } *** { http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1238759731.1238763072.19972.gz OS X 10.5.2 mozilla-1.9.1 unit test on 2009/04/03 04:55:31 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 7951 bytes during test execution TinderboxPrint: mochitest-plain<br/>76129/0/1541&nbsp;<em class="testfail">LEAK</em> }
Flags: wanted-firefox3.5?
OS: Linux → All
Summary: mochitest-chrome intermittent leak of 8 kB → mochitest-m* intermittent leak of 8 kB
{ http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1238800317.1238805692.10732.gz OS X 10.5.2 mozilla-central unit test on 2009/04/03 16:11:57 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6047 bytes during test execution TinderboxPrint: mochitest-browser-chrome<br/>2106/0/5&nbsp;<em class="testfail">LEAK</em> }
Summary: mochitest-m* intermittent leak of 8 kB → mochitest-*: intermittent leak of 6-8 kB
Version: 3.1 Branch → Trunk
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239155345.1239158820.2148.gz OS X 10.5.2 mozilla-central unit test on 2009/04/07 18:49:05 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239180376.1239186312.26222.gz Linux mozilla-central unit test on 2009/04/08 01:46:16 mochitest-chrome http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239198393.1239205240.2634.gz Linux mozilla-central unit test on 2009/04/08 06:46:33 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1239224983.1239230441.21568.gz Linux mozilla-1.9.1 unit test on 2009/04/08 14:09:43 mochitest-browser-chrome
Depends on: 462410
No longer depends on: 472677
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1239391089.1239400413.31343.gz WINNT 5.2 mozilla-1.9.1 unit test on 2009/04/10 12:18:09 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6691 bytes during test execution mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239499976.1239506405.18991.gz Linux mozilla-central unit test on 2009/04/11 18:32:56 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6215 bytes during test execution mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239595870.1239601486.22877.gz OS X 10.5.2 mozilla-central unit test on 2009/04/12 21:11:10 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239611921.1239616041.13160.gz TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 7095 bytes during test execution OS X 10.5.2 mozilla-central unit test on 2009/04/13 01:38:41 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239624155.1239633551.18748.gz WINNT 5.2 mozilla-central unit test mochitest-chrome I'm not sure this should be a single bug report, though; there might be slight differences in the list of objects that are key, but that all trigger a bunch of services to be leaked. We might want to see what the differences in the list of objects leaked are.
I agree, but noone seems particularly interested in bug 472677 or this one yet...
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239689562.1239692898.1648.gz OS X 10.5.2 mozilla-central unit test on 2009/04/13 23:12:42 mochitest-plain
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1239820264.1239827764.19952.gz WINNT 5.2 mozilla-1.9.1 unit test on 2009/04/15 11:31:04 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1239829912.1239835011.32134.gz Linux mozilla-1.9.1 unit test on 2009/04/15 14:11:52 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239993210.1239997676.7459.gz OS X 10.5.2 mozilla-central unit test on 2009/04/17 11:33:30 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240019814.1240023411.12868.gz OS X 10.5.2 mozilla-central unit test on 2009/04/17 18:56:54
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1238660210.1238670125.576.gz Linux mozilla-1.9.1 unit test on 2009/04/02 01:16:50 If we wanted to diagnose what's leaking, we could potentially land http://hg.mozilla.org/users/dbaron_mozilla.com/patches/file/tip/dump-leaked-urls-at-shutdown temporarily (with different ifdefs), assuming the URLs leaked are related to the HTTP connections leaked.
Summary: mochitest-*: intermittent leak of 6-8 kB → mochitest-*: intermittent leak of 6-8 kB including nsHttpConnection, nsUrlClassifier*
Er, sorry, that was a paste of the original log; I meant to paste: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240254806.1240264393.21236.gz WINNT 5.2 mozilla-central unit test on 2009/04/20 12:13:26
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1240392190.1240399404.15579.gz OS X 10.5.2 mozilla-1.9.1 unit test on 2009/04/22 02:23:10 mochitest-browser-chrome
Keywords: helpwanted
Whiteboard: [orange] → [ToDo: comment 17] [orange]
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240567604.1240573788.1117.gz Linux mozilla-central unit test on 2009/04/24 03:06:44 mochitest-browser-chrome
(previous one was mochitest-chrome) http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240580435.1240584818.23444.gz OS X 10.5.2 mozilla-central unit test on 2009/04/24 06:40:35 mochitest-browser-chrome http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240580435.1240590126.2563.gz WINNT 5.2 mozilla-central unit test on 2009/04/24 06:40:35 mochitest-plain
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1240564941.1240573845.1205.gz OS X 10.5.2 mozilla-central unit test on 2009/04/24 02:22:21 mochitest-browser-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1241467939.1241470888.23549.gz OS X 10.5.2 mozilla-central unit test on 2009/05/04 13:12:19 mochitest-chrome
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1241533446.1241536914.25523.gz OS X 10.5.2 mozilla-1.9.1 unit test on 2009/05/05 07:24:06
Component: General → Phishing Protection
QA Contact: general → phishing.protection
(In reply to comment #32) > Interestingly enough, this one happened while > http://hg.mozilla.org/mozilla-central/rev/240fa475d2b0 was in the tree, so we Good :-) For next time, could you update the output format so it ends up in the brief report (when there are leaks)? > now know that the two leaked nsStandardURL objects were: Just to confirm, are we sure it's always these urls which are leaked?
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1242089048.1242093666.13875.gz OS X 10.5.2 mozilla-central unit test on 2009/05/11 17:44:08
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242344679.1242351604.26443.gz OS X 10.5.2 mozilla-1.9.1 unit test on 2009/05/14 16:44:39
Fwiw, it seems the nsHttpConnection instance stopped being leaked on 2009/04/03.
Summary: mochitest-*: intermittent leak of 6-8 kB including nsHttpConnection, nsUrlClassifier* → mochitest-*: intermittent leak of 6-8 kB including nsHttpConnectionInfo, nsUrlClassifier*
Flags: blocking-firefox3.5?
Whiteboard: [ToDo: comment 17] [orange] → [ToDo: look into comment 17+32+33] [orange]
Not a blocker for Firefox 3.5 unless we think it's contributing to a leak that's accumulating over time to become something huge ...
Flags: blocking-firefox3.5? → blocking-firefox3.5-
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1242816875.1242825505.31039.gz WINNT 5.2 mozilla-1.9.1 unit test on 2009/05/20 03:54:35
(In reply to comment #10) > I'm not sure this should be a single bug report, though; there might be slight > differences in the list of objects that are key, but that all trigger a bunch > of services to be leaked. We might want to see what the differences in the > list of objects leaked are. (In reply to comment #39) > Fwiw, it seems the nsHttpConnection instance stopped being leaked on > 2009/04/03. Actually, it does seem there are two versions of the same(!?) leak: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1243108189.1243113754.18543.gz Linux mozilla-1.9.1 unit test on 2009/05/23 12:49:49 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 8515 bytes during test execution mochitest-plain with a nsHttpConnection leak. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1243115389.1243119750.28917.gz Linux mozilla-1.9.1 unit test on 2009/05/23 14:49:49 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6599 bytes during test execution mochitest-browser-chrome without a nsHttpConnection leak.
Whiteboard: [ToDo: look into comment 17+32+33] [orange] → [ToDo: look into comment 17+32+33+49] [orange]
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.5/1243115389.1243121927.31843.gz OS X 10.5.2 mozilla-1.9.1 unit test on 2009/05/23 14:49:49 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6327 bytes during test execution mochitest-browser-chrome without a nsHttpConnection leak.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243348533.1243352795.2017.gz OS X 10.5.2 mozilla-central unit test on 2009/05/26 07:35:33 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 7135 bytes during test execution mochitest-browser-chrome with a nsHttpConnection leak.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243429710.1243434087.18669.gz OS X 10.5.2 mozilla-central unit test on 2009/05/27 06:08:30 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 6087 bytes during test execution mochitest-browser-chrome without a nsHttpConnection leak.
(In reply to comment #54) > http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1243558960.1243568421.16527.gz WINNT 5.2 mozilla-central unit test on 2009/05/28 18:02:40 TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 7471 bytes during test execution mochitest-browser-chrome with a nsHttpConnection leak.
Whiteboard: [ToDo: look into comment 17+32+33+49] [orange] → [Don't add new log] [ToDo: look into comment 17+32+33+49] [orange]
That's not too surprising, for a urlclassifier leak, I guess. Hmm. I don't think we want to turn off safebrowsing in tests (we have safebrowsing tests, after all!) but I wonder if we can do something to test locally, or otherwise mitigate this. Or maybe we should just take more care to shutdown cleanly and not leak these, if we get closed mid-update. I don't remember if sdwilsh ended up handing off safebrowsing to ddahl or not, but cc'ng them both anyhow.
Updating whiteboard - I think comment #56 strikes to the heart of the problem, so the previous whiteboard suggestion to look at earlier comments is less relevant.
Whiteboard: [Don't add new log] [ToDo: look into comment 17+32+33+49] [orange] → [orange]
(In reply to comment #56) > The new logging is showing the following URLs leaked: These 2 urls seem to be the "same" as in comment 32. (which is a good confirmation.)
Assignee: nobody → dtownsend
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244719127.1244725250.6220.gz&fulltext=1 WINNT 5.2 mozilla-central unit test on 2009/06/11 04:18:47 mochitest-browser-chrome same 2 urls, without a nsHttpConnection leak.
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244730758.1244736342.866.gz&fulltext=1 WINNT 5.2 mozilla-central unit test on 2009/06/11 07:32:38 mochitest-browser-chrome same 2 urls, without a nsHttpConnection leak.
I'm pretty sure we still don't need new logs here
Whiteboard: [orange] → [orange] [Don't add new log]
Blocks: 462410
No longer depends on: 462410
Let's start by turning on the automated tests that have been slacking off and not running since checkin
Attachment #382940 - Flags: review?(ted.mielczarek)
Attachment #382940 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 382940 [details] [diff] [review] turn on tests [checked in] Landed on trunk: http://hg.mozilla.org/mozilla-central/rev/3dbd14e803db
Attachment #382940 - Attachment description: turn on tests → turn on tests [checked in]
While we can wallpaper over this problem quite easily it would be better to be sure of exactly what it is first. This patch turns on logging for safebrowsing on the unit test boxes. I'd like to land it on m-c for a short time to see what it shows for when we leak. Given how often this happens I imagine it wouldn't need to be in long. The downside is that it could spew quite a lot into the build log, even interspersed with other test results which might make for confusion.
Attachment #382958 - Flags: review?(ted.mielczarek)
Attachment #382958 - Flags: review?(ted.mielczarek) → review+
Depends on: 497884
Checked in the logging patch and added a note about it to tinderbox (http://hg.mozilla.org/mozilla-central/rev/9a72a7036822). Please do add logs here if I don't spot them
Whiteboard: [orange] [Don't add new log] → [orange]
So did this logging tell you anything you didn't already know? The fix in bug 497884 should fix this, right? (The root cause was the background update of the databases, AIUI.)
Also, another vote for bug 472007.
(In reply to comment #72) > So did this logging tell you anything you didn't already know? The fix in bug > 497884 should fix this, right? (The root cause was the background update of the > databases, AIUI.) Yeah bug 497884 should mask this on tinderbox, I'll drop this off the orange list once that has landed on branch too. With everything I've investigated so far I still haven't been able to narrow down exactly what is happening here so I can't reproduce it. I'm pretty sure that the is a bug here, but don't believe it is serious and it's unlikely I'll be dedicating much time to it once this is no longer showing up randomly orange. From what I can tell certain tests have the effect of making the http channel that is pulling data from Google to stop delivering data, mid-stream, but not close the channel. This wouldn't normally be a problem, during shutdown the channel is closed and this is happening in the leaking cases, but for some reason the channel stays alive past that, which keeps everything else alive. This seems to happen quite a lot around the tests testing the private browsing which makes me wonder if something about those tests is screwing up the networking, also around tests that send the browser offline. But all of that may just be coincidental, we'd have to do the logging for a much longer period to determine any kind of pattern. Unfortunately the individual connections to Google are very fast so it is pretty much impossible to manually trigger tests at the right moment to catch them.
(In reply to comment #74) > From what I can tell certain tests have the effect of making the http channel > that is pulling data from Google to stop delivering data, mid-stream, but not > close the channel. This wouldn't normally be a problem, during shutdown the > channel is closed and this is happening in the leaking cases, but for some > reason the channel stays alive past that, which keeps everything else alive. > > This seems to happen quite a lot around the tests testing the private browsing > which makes me wonder if something about those tests is screwing up the > networking, also around tests that send the browser offline. But all of that > may just be coincidental I bet not! Transitioning into/out of private browsing resets Necko - I rather suspect offline mode tests would too. Perhaps that code path is not good about cleaning up stray channels? Ehsan's working on having private browsing NOT do that over in bug 496335.
Depends on: 472007
Depends on: 496335
Here's a guess: As I recall, URL classifier suspends the necko channel. Is it possible that it never resumes it? Suspended channels must be resumed even if they get cancelled.
(In reply to comment #76) > Here's a guess: As I recall, URL classifier suspends the necko channel. No there is no suspending of the channel going on from the url-classifier code that I can see.
Attachment #382958 - Attachment description: additional logging → additional logging [Backout: Comment 71]
(In reply to comment #78) > http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1245281675.1245287790.31828.gz > Linux try hg unit test on 2009/06/17 16:34:35 How do you tell which revision this build was based on? The 'hg identify' (and other) commands don't log anything :-/ By time, it would be after bug 497884 fix, I think; but "added 28995 changesets" looks like it was before (r.29285) actually.
(In reply to comment #79) > (In reply to comment #78) > How do you tell which revision this build was based on? If they build was generated via "push-to-try", you can identify it by searching for the build-submitter's email address, getting the changeset ID on the line below that, and then looking that up on http://hg.mozilla.org/try/pushloghtml e.g. in the following log, it mentions changeset ID e57add7be3f, and the "try" pushlog shows that the last non-testing ancestor of that is d159361bf624. http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1245283085.1245291997.5866.gz&fulltext=1 If the build was generated from the web try-server interface (as the one you quoted seems to be), it won't include a changeset ID -- it has the submitter's chosen label, instead. ("variable_LIR" in the log you quoted). It also won't be mentioned on the try-server pushlog. I think your best bet there is to just guess based on build start-time.
(In reply to comment #78) > http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1245281675.1245287790.31828.gz > Linux try hg unit test on 2009/06/17 16:34:35 This is a build from the tracemonkey repo, they haven't merged with m-c since 5th of June so they haven't got the fix for bug 497884
(In reply to comment #77) > (In reply to comment #76) > > Here's a guess: As I recall, URL classifier suspends the necko channel. > > No there is no suspending of the channel going on from the url-classifier code > that I can see. Hm right, the suspending happens in the caller (http://mxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#9622)
(In reply to comment #82) > (In reply to comment #77) > > (In reply to comment #76) > > > Here's a guess: As I recall, URL classifier suspends the necko channel. > > > > No there is no suspending of the channel going on from the url-classifier code > > that I can see. > > Hm right, the suspending happens in the caller > (http://mxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#9622) That's the page load request right? All the logs are suggesting that it is actually the download of blocklist data from google (which happens in the background unaffected by page load) that is getting stuck.
(In reply to comment #83) > That's the page load request right? All the logs are suggesting that it is > actually the download of blocklist data from google (which happens in the > background unaffected by page load) that is getting stuck. Ah, sorry for missing that. I have no theory about that one.
(In reply to comment #80) > If they build was generated via "push-to-try", you can identify it by searching > for the build-submitter's email address, getting the changeset ID on the line > below that, and then looking that up on http://hg.mozilla.org/try/pushloghtml > > e.g. in the following log, it mentions changeset ID e57add7be3f, and the "try" > pushlog shows that the last non-testing ancestor of that is d159361bf624. > http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1245283085.1245291997.5866.gz&fulltext=1 Right, this case is obvious :-) > If the build was generated from the web try-server interface (as the one you > quoted seems to be), it won't include a changeset ID -- it has the submitter's > chosen label, instead. ("variable_LIR" in the log you quoted). It also won't > be mentioned on the try-server pushlog. I think your best bet there is to just > guess based on build start-time. (Wow, room for improvement...) Actually, (I assume) this was based on f7c6c9e6e44d: Bug 498926 - lirasm - Support --execute with LIR_fret. Paritosh Aggarwal <contactparitosh@gmail.com> - Wed, 17 Jun 2009 11:52:20 -0500 - rev 28994 (In reply to comment #81) > This is a build from the tracemonkey repo, they haven't merged with m-c since (Oh, I didn't expect non m-c builds :-< Now, I'll know.) > 5th of June so they haven't got the fix for bug 497884 Indeed, http://hg.mozilla.org/tracemonkey/annotate/f7c6c9e6e44d/build/automation.py.in doesn't have the blocking fix. Then, basically, this build was "expected" to fail and should (have) be ignored.
Blocks: 472677
Removing the dependencies on the automated test issues since this should not longer be showing up in automated tests on 1.9.1 or trunk and it will just cloud things I think.
Assignee: dtownsend → nobody
No longer blocks: mlkTests, 438871, 472677
No longer depends on: 472007
Whiteboard: [orange]
Based on comment 75, now that bug 496335 has been fixed, can this be closed?
Iiuc, (In reply to comment #74) > Yeah bug 497884 should mask this on tinderbox, I'll drop this off the orange > list once that has landed on branch too. This was a workaround: no report of this bug since then. (In reply to comment #87) > Based on comment 75, now that bug 496335 has been fixed, can this be closed? And this eventually fixed the cause.
Blocks: mlkTests
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: wanted-firefox3.5?
Keywords: helpwanted
Resolution: --- → FIXED
Whiteboard: [Fixed by bug 497884 and bug 496335] [orange]
Target Milestone: --- → Firefox 3.6a1
Whiteboard: [Fixed by bug 497884 and bug 496335] [orange] → [Fixed by bug 497884 and bug 496335]
(In reply to comment #33) > (In reply to comment #32) > > > Interestingly enough, this one happened while > > http://hg.mozilla.org/mozilla-central/rev/240fa475d2b0 was in the tree, so we > > Good :-) > > For next time, could you update the output format so it ends up in the brief > report (when there are leaks)? > > > now know that the two leaked nsStandardURL objects were: > > Just to confirm, are we sure it's always these urls which are leaked?
(In reply to comment #89) Click at the wrong place, sorry. (In reply to comment #33) > For next time, could you update the output format so it ends up in the brief > report (when there are leaks)? I filed bug 512161.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: