777044 - networking code has reference cycles; doesn't appear to declare them to the CC; this might be fine of course, just checking

Status: RESOLVED INVALID

(Core :: Networking: HTTP, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

There don't appear to be any CYCLE_COLLECTING macros in the netwerk/ code:

$ grep -R CYCLE netwerk/ | wc -l
0

Yet, running a tool that finds reference cycles, I find many reference cycles in the netwerk code, most of them falling into either of the following two patterns (by patterns I mean the set of places in the source code where the referenced objects are allocated).

Please have a look; of course this might be just fine if you're explicitly destroying these objects/references.

Pattern #1:

Contains 2 blocks, listed below. The references are a nsCOMPtr in one way, and a nsRefPtr in the other way.

block 618466
has references to these blocks: 12696, 530459
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x69704C0: nsInputStreamPump::Create(nsInputStreamPump**, nsIInputStream*, long, long, unsigned int, unsigned int, bool) (mozalloc.h:200)
by 0x6A2B856: mozilla::net::nsHttpChannel::SetupTransaction() (nsHttpChannel.cpp:866)
by 0x6A30E17: mozilla::net::nsHttpChannel::ContinueConnect() (nsHttpChannel.cpp:506)
by 0x6A35E83: mozilla::net::nsHttpChannel::OnCacheEntryAvailable(nsICacheEntryDescriptor*, int, unsigned int) (nsHttpChannel.cpp:5454)
by 0x6A2E2F7: mozilla::net::HttpCacheQuery::Run() (nsHttpChannel.cpp:2882)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x783433F: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:82)
by 0x79DDB5A: MessageLoop::RunInternal() (message_loop.cc:208)
by 0x79DDB8B: MessageLoop::Run() (message_loop.cc:175)

block 530459
has references to these blocks: 270246, 530455, 530467, 530470, 530471, 618452, 618466
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6A1B659: nsHttpHandler::NewProxiedChannel(nsIURI*, nsIProxyInfo*, nsIChannel**) (mozalloc.h:200)
by 0x6A196A5: nsHttpHandler::NewChannel(nsIURI*, nsIChannel**) (nsHttpHandler.cpp:1398)
by 0x6976A2D: nsIOService::NewChannelFromURIWithProxyFlags(nsIURI*, nsIURI*, unsigned int, nsIChannel**) (nsIOService.cpp:657)
by 0x696E061: NS_NewChannel(nsIChannel**, nsIURI*, nsIIOService*, nsILoadGroup*, nsIInterfaceRequestor*, unsigned int, nsIChannelPolicy*) (nsNetUtil.h:191)
by 0x6E35C9F: nsScriptLoader::StartLoad(nsScriptLoadRequest*, nsAString_internal const&) (nsScriptLoader.cpp:297)
by 0x6E3840C: nsScriptLoader::ProcessScriptElement(nsIScriptElement*) (nsScriptLoader.cpp:554)
by 0x6E3416B: nsScriptElement::MaybeProcessScript() (nsScriptElement.cpp:136)
by 0x6F4D085: nsHTMLScriptElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) (nsHTMLScriptElement.cpp:150)
by 0x6E152C6: nsINode::doInsertChildAt(nsIContent*, unsigned int, bool, nsAttrAndChildArray&) (nsINode.cpp:1304)
by 0x6E16ED8: nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*) (nsINode.cpp:1886)



Pattern #2:


Contains 2 blocks, listed below. The references are nsCOMPtrs.

block 416833
has references to these blocks: 12696, 396050
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6A0DF03: nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream*) (mozalloc.h:200)
by 0x6990A6D: nsSocketOutputStream::OnSocketReady(unsigned int) (nsSocketTransport2.cpp:490)
by 0x6990B0E: nsSocketTransport::OnSocketReady(PRFileDesc*, short) (nsSocketTransport2.cpp:1532)
by 0x69954EA: nsSocketTransportService::DoPollIteration(bool) (nsSocketTransportService2.cpp:743)
by 0x69956FC: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:614)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x79AA5D0: nsThread::ThreadFunc(void*) (nsThread.cpp:257)
by 0x4085FFA: _pt_root (ptthread.c:156)
by 0x4A2CE99: start_thread (pthread_create.c:308)

block 396050
has references to these blocks: 416414, 416833
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6993A0C: nsSocketTransportService::CreateTransport(char const**, unsigned int, nsACString_internal const&, int, nsIProxyInfo*, nsISocketTransport**) (mozalloc.h:200)
by 0x6A0A2D7: nsHttpConnectionMgr::nsHalfOpenSocket::SetupStreams(nsISocketTransport**, nsIAsyncInputStream**, nsIAsyncOutputStream**, bool) (nsHttpConnectionMgr.cpp:2388)
by 0x6A0AF61: nsHttpConnectionMgr::nsHalfOpenSocket::SetupPrimaryStreams() (nsHttpConnectionMgr.cpp:2450)
by 0x6A0D357: nsHttpConnectionMgr::CreateTransport(nsHttpConnectionMgr::nsConnectionEntry*, nsAHttpTransaction*, unsigned char, bool) (nsHttpConnectionMgr.cpp:1759)
by 0x6A0ECE6: nsHttpConnectionMgr::OnMsgSpeculativeConnect(int, void*) (nsHttpConnectionMgr.cpp:2284)
by 0x6A05C35: nsHttpConnectionMgr::nsConnEvent::Run() (nsHttpConnectionMgr.h:538)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x6995723: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:621)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

By the way, the tool that found this is attached to bug 704240.

Comment 2

[Andrew McCreight [:mccr8]]

I don't know anything about the particulars here, but my understanding is that networking code can manipulate stuff off the main thread, and the cycle collector can't be used on non-main-thread objects, so networking has to deal with cycles itself.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> I don't know anything about the particulars here, but my understanding is
> that networking code can manipulate stuff off the main thread, and the cycle
> collector can't be used on non-main-thread objects, so networking has to
> deal with cycles itself.

For example, nsHttpChannel <-> HttpCacheQuery. There is a point where there is a cycle, and the cycle collector can't be used (AFAICT) because of threading issues. Instead, we rely on the fact that the cache query will eventually finish, and as part of the finishing, it releases its reference to the nsHttpChannel.

If there is a way of annotating the references to eliminate false-positives then we should use that mechanism in Necko. Is there such a mechanism?

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

The tool that made this report is very new and experimental and doesn't have a way to read any such annotation.

I'm just curious: are these cycles OK ?

Comment 5

[Jason Duell]

There are definitely times when it's ok to see cycles in necko--for instance, channels hold a ref to their listeners in between asyncOpen and OnStopRequest.  After that they are guaranteed (unless we have bugs) to release the listener ref.  Meanwhile the listener can keep a ref to the channel for an arbitrary period, but after OnStopRequest the cycle is broken.

Are the two cycles you found in your report the only ones you're seeing?  We should check them out to make sure that code is breaking the cycle in all cases.

I assume your tool shows cycles at a particular moment in time, right?  Could be worth doing lots of browsing, then waiting for a quiet moment to run it, and then we should see if any necko cycles haven't been broken.   Or it that what you did already?

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Jason Duell (:jduell) from comment #5)
> Are the two cycles you found in your report the only ones you're seeing?  We
> should check them out to make sure that code is breaking the cycle in all
> cases.

They are the two only cycles that fell entirely within the netwerk/ code. There were more cycles involving both netwerk and non-netwerk (image) code. Pasting one below.

> 
> I assume your tool shows cycles at a particular moment in time, right? 

Right.

> Could be worth doing lots of browsing, then waiting for a quiet moment to
> run it, and then we should see if any necko cycles haven't been broken.   Or
> it that what you did already?

I can't do that yet as the current implementation of the tool only allows to get the graph once per application lifetime, but I plan for that to be possible in the future.

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Here's the cycle mixing netwerk and image, let's call it Pattern #3:

Contains 9 blocks, listed below:

block 622978
has references to these blocks: 559400
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x79B8ED0: NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor*, nsIInterfaceRequestor*, nsIInterfaceRequestor**) (mozalloc.h:200)
by 0x6A29906: NS_NewNotificationCallbacksAggregation(nsIInterfaceRequestor*, nsILoadGroup*, nsIInterfaceRequestor**) (nsNetUtil.h:1412)
by 0x6A2B5E1: mozilla::net::nsHttpChannel::SetupTransaction() (nsHttpChannel.cpp:825)
by 0x6A30E17: mozilla::net::nsHttpChannel::ContinueConnect() (nsHttpChannel.cpp:506)
by 0x6A35E83: mozilla::net::nsHttpChannel::OnCacheEntryAvailable(nsICacheEntryDescriptor*, int, unsigned int) (nsHttpChannel.cpp:5454)
by 0x6A2E2F7: mozilla::net::HttpCacheQuery::Run() (nsHttpChannel.cpp:2882)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x783433F: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:82)
by 0x79DDB5A: MessageLoop::RunInternal() (message_loop.cc:208)

block 559400
has references to these blocks: 559375, 559398
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6AF9E0A: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (mozalloc.h:200)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)
by 0x6C6F9B9: nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const (nsCSSDataBlock.cpp:116)
by 0x6CDC7B7: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) (nsRuleNode.cpp:1860)
by 0x6CDE3AC: nsRuleNode::GetStyleBackground(nsStyleContext*, bool) (nsStyleStructList.h:79)
by 0x6B40ADD: nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:5506)
by 0x6B40C62: nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:9739)

block 622979
has references to these blocks: 12696, 622978, 622980
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6A2B60C: mozilla::net::nsHttpChannel::SetupTransaction() (mozalloc.h:200)
by 0x6A30E17: mozilla::net::nsHttpChannel::ContinueConnect() (nsHttpChannel.cpp:506)
by 0x6A35E83: mozilla::net::nsHttpChannel::OnCacheEntryAvailable(nsICacheEntryDescriptor*, int, unsigned int) (nsHttpChannel.cpp:5454)
by 0x6A2E2F7: mozilla::net::HttpCacheQuery::Run() (nsHttpChannel.cpp:2882)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x783433F: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:82)
by 0x79DDB5A: MessageLoop::RunInternal() (message_loop.cc:208)
by 0x79DDB8B: MessageLoop::Run() (message_loop.cc:175)
by 0x7742270: nsBaseAppShell::Run() (nsBaseAppShell.cpp:163)

block 622993
has references to these blocks: 12696, 559363
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x69704C0: nsInputStreamPump::Create(nsInputStreamPump**, nsIInputStream*, long, long, unsigned int, unsigned int, bool) (mozalloc.h:200)
by 0x6A2B856: mozilla::net::nsHttpChannel::SetupTransaction() (nsHttpChannel.cpp:866)
by 0x6A30E17: mozilla::net::nsHttpChannel::ContinueConnect() (nsHttpChannel.cpp:506)
by 0x6A35E83: mozilla::net::nsHttpChannel::OnCacheEntryAvailable(nsICacheEntryDescriptor*, int, unsigned int) (nsHttpChannel.cpp:5454)
by 0x6A2E2F7: mozilla::net::HttpCacheQuery::Run() (nsHttpChannel.cpp:2882)
by 0x79AAF24: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:624)
by 0x7963E60: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:217)
by 0x783433F: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (MessagePump.cpp:82)
by 0x79DDB5A: MessageLoop::RunInternal() (message_loop.cc:208)
by 0x79DDB8B: MessageLoop::Run() (message_loop.cc:175)

block 559375
has references to these blocks: 270251, 500522, 559359, 559363, 559377
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6AF294E: NewRequestAndEntry(bool, imgRequest**, imgCacheEntry**) (mozalloc.h:200)
by 0x6AF9868: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (imgLoader.cpp:1657)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)
by 0x6C6F9B9: nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const (nsCSSDataBlock.cpp:116)
by 0x6CDC7B7: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) (nsRuleNode.cpp:1860)
by 0x6CDE3AC: nsRuleNode::GetStyleBackground(nsStyleContext*, bool) (nsStyleStructList.h:79)
by 0x6B40ADD: nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:5506)

block 559377
has references to these blocks: 559375
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6AF297B: NewRequestAndEntry(bool, imgRequest**, imgCacheEntry**) (mozalloc.h:200)
by 0x6AF9868: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (imgLoader.cpp:1657)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)
by 0x6C6F9B9: nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const (nsCSSDataBlock.cpp:116)
by 0x6CDC7B7: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) (nsRuleNode.cpp:1860)
by 0x6CDE3AC: nsRuleNode::GetStyleBackground(nsStyleContext*, bool) (nsStyleStructList.h:79)
by 0x6B40ADD: nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:5506)

block 559363
has references to these blocks: 559359, 559371, 559374, 559383, 559400, 622979, 622993
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6A1B659: nsHttpHandler::NewProxiedChannel(nsIURI*, nsIProxyInfo*, nsIChannel**) (mozalloc.h:200)
by 0x6A196A5: nsHttpHandler::NewChannel(nsIURI*, nsIChannel**) (nsHttpHandler.cpp:1398)
by 0x6976A2D: nsIOService::NewChannelFromURIWithProxyFlags(nsIURI*, nsIURI*, unsigned int, nsIChannel**) (nsIOService.cpp:657)
by 0x696E061: NS_NewChannel(nsIChannel**, nsIURI*, nsIIOService*, nsILoadGroup*, nsIInterfaceRequestor*, unsigned int, nsIChannelPolicy*) (nsNetUtil.h:191)
by 0x6AF4118: NewImageChannel(nsIChannel**, bool*, nsIURI*, nsIURI*, nsIURI*, nsILoadGroup*, nsCString const&, unsigned int, nsIChannelPolicy*, nsIPrincipal*) (imgLoader.cpp:510)
by 0x6AF97EA: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (imgLoader.cpp:1652)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)

block 559398
has references to these blocks: 270246, 559359, 559375
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6AF3C2F: imgLoader::CreateNewProxyForRequest(imgRequest*, nsILoadGroup*, imgIDecoderObserver*, unsigned int, imgIRequest*, imgIRequest**) (mozalloc.h:200)
by 0x6AF9DD0: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (imgLoader.cpp:1739)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)
by 0x6C6F9B9: nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const (nsCSSDataBlock.cpp:116)
by 0x6CDC7B7: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) (nsRuleNode.cpp:1860)
by 0x6CDE3AC: nsRuleNode::GetStyleBackground(nsStyleContext*, bool) (nsStyleStructList.h:79)
by 0x6B40ADD: nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:5506)

block 559383
has references to these blocks: 559375
allocation call stack:
at 0x402B1FE: malloc (vg_replace_malloc.c:268)
by 0x403BEE4: moz_xmalloc (mozalloc.cpp:54)
by 0x6AF99FA: imgLoader::LoadImage(nsIURI*, nsIURI*, nsIURI*, nsIPrincipal*, nsILoadGroup*, imgIDecoderObserver*, nsISupports*, unsigned int, nsISupports*, imgIRequest*, nsIChannelPolicy*, imgIRequest**) (mozalloc.h:200)
by 0x6DA5809: nsContentUtils::LoadImage(nsIURI*, nsIDocument*, nsIPrincipal*, nsIURI*, imgIDecoderObserver*, int, imgIRequest**) (nsContentUtils.cpp:2697)
by 0x6CAF0D7: nsCSSValue::Image::Image(nsIURI*, nsStringBuffer*, nsIURI*, nsIPrincipal*, nsIDocument*) (nsCSSValue.cpp:1724)
by 0x6CAFFC9: nsCSSValue::StartImageLoad(nsIDocument*) const (nsCSSValue.cpp:570)
by 0x6C6E9B1: TryToStartImageLoad(nsCSSValue const&, nsIDocument*, nsCSSProperty) (nsCSSDataBlock.cpp:69)
by 0x6C6F9B9: nsCSSCompressedDataBlock::MapRuleInfoInto(nsRuleData*) const (nsCSSDataBlock.cpp:116)
by 0x6CDC7B7: nsRuleNode::WalkRuleTree(nsStyleStructID, nsStyleContext*) (nsRuleNode.cpp:1860)
by 0x6CDE3AC: nsRuleNode::GetStyleBackground(nsStyleContext*, bool) (nsStyleStructList.h:79)
by 0x6B40ADD: nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:5506)
by 0x6B40C62: nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsIFrame*, nsFrameItems&) (nsCSSFrameConstructor.cpp:9739)

Comment 8

[Jason Duell]

> tool only allows to get the graph once per application lifetime, 

OK.  When are you grabbing the graph now?  For this bug, I'd recommend you do it at a point when all network traffic should be quiet, so we can avoid the temporary cycles we know exist during resource downloads.

Thanks for looking into this, Benoit!  I'm guessing most of these aren't real leak, but if we find one or two real ones that would be huge.

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

That was grabbed during page load. I've redone it now, waiting a couple minutes after the pages were apparently finished loading, and got this graph: (gzipped plain text):

  https://bugzilla.mozilla.org/attachment.cgi?id=645572

Indeed, pattern #1 has disappeared, so it seems confirmed that pattern #1 was not a leak. Ditto for pattern #3.

But pattern #2 is still there, so maybe it is worth looking specifically into this one and checking if it's not a leak. Unfortunately, the tool is not yet able to verify if anything still has a reference to these objects.

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Ping about pattern #2: is there anything I can do to help verify if it's normal that these cycles stay around after the end of page load?

Comment 11

[Jason Duell]

OK, so case #2 (the only remaining cycle) is a ref cycle between a nsISocketTransport and something allocated in nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady, IIUC.   I see both a nsHttpConnection and NullHttpTransaction created there--am I right that it's not clear from the stack which object is being allocated?  (any reason we can't get a stack with source file line #s for that call?).

Note that we keep connections alive for a while (Patrick--how long?) before we expire them (with a timer I presume), so might be worth waiting a little longer and seeing if there's still a cycle...

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

Indeed it's not fully clear because OnOutputStreamReady is apparently inlined. My problem is I pretty much have to enable optimization to get workable performance in Valgrind, and then inlining hurts the stacks. I tried -fno-inline but some functions remained inlined.

I'm working on a new version of the tool that doesn't rely on Valgrind so it'll be easier to get more data. If Patrick can say exactly how long to wait before taking a snapshot, I can still retry with the current tool, though.

Comment 13

[Patrick McManus [:mcmanus]]

(In reply to Benoit Jacob [:bjacob] from comment #12)

> I'm working on a new version of the tool that doesn't rely on Valgrind so
> it'll be easier to get more data. If Patrick can say exactly how long to
> wait before taking a snapshot, I can still retry with the current tool,
> though.

idle connections can stay connected for up to 3 minutes.
unused idle connections only stay alive for 5 seconds
half opens that are working on a connection shouldn't be around for more than 2 minutes (on current nightly)

Comment 14

[Nicholas Nethercote [inactive]]

Removing the Memshrink tag until we have stronger evidence that this is a genuine problem.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Is there anything actionable here?

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

There was, and with an improved version of the refgraph tool I verified that the cycles mentioned above weren't actually leaked --- they eventually did go away.