Reference cycle between imgRequest and nsFileChannel is not declared to XPCOM cycle collector

REOPENED
Unassigned

Status

()

REOPENED
7 years ago
6 years ago

People

(Reporter: bjacob, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Using the XPCOM reference tracing patch in bug 704240 I saw this:

REFTRACING: nsCOMPtr@0x1f324f8 : imgRequest@0x1f323a0 -> nsFileChannel@0x1f31cb0
REFTRACING: nsCOMPtr@0x1f324c8 : imgRequest@0x1f323a0 -> nsSystemPrincipal@0x1894580
REFTRACING: nsCOMPtr@0x1f31e30 : nsFileChannel@0x1f31cb0 -> imgRequest@0x1f323a0

This means that a imgRequest and a nsFileChannel are referencing each other. Looking at imgRequest.cpp and nsFileChannel.cpp, I can't find where this cycle is declared to the cycle collector?

I didn't observe an actual leak though, as these objects got deleted soon after.
(Reporter)

Comment 1

7 years ago
Still using that patch, I do see a large number of imgRequest objects being kept alive until very late during Firefox shutdown, not sure if that's expected.
imgRequest removes its reference to the channel after OnStopRequest, so it's not actually an issue. And yes, a large number of imgRequest objects are stored until shutdown: that's the image cache! :)

For now I'm going to call this invalid, but please reopen if necessary.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → INVALID
(Reporter)

Comment 3

7 years ago
oh ok, thanks!
Note that imgRequest and imgRequestProxy are not cycle collected at all, which in fact has caused leak problems....

We should really consider fixing that.  :(
(Reporter)

Comment 5

7 years ago
reopening then! can't deny i'm glad that my tool has found something.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(Reporter)

Comment 6

7 years ago
Created attachment 576214 [details] [diff] [review]
patch that fails to build

Can someone teach me how to get this to build?

/home/bjacob/mozilla-central/image/src/imgRequest.cpp: In member function ‘virtual nsrefcnt imgRequest::AddRef()’:
/home/bjacob/mozilla-central/image/src/imgRequest.cpp:177:1: error: no match for ‘operator++’ in ‘++((imgRequest*)this)->imgRequest::mRefCnt’
/home/bjacob/mozilla-central/image/src/imgRequest.cpp:177:1: note: candidate is:
../../dist/include/gfxRect.h:83:36: note: mozilla::css::Corner operator++(mozilla::css::Corner&, int)
../../dist/include/gfxRect.h:83:36: note:   candidate expects 2 arguments, 1 provided
/home/bjacob/mozilla-central/image/src/imgRequest.cpp: In member function ‘virtual nsrefcnt imgRequest::Release()’:
/home/bjacob/mozilla-central/image/src/imgRequest.cpp:177:1: error: no match for ‘operator--’ in ‘--((imgRequest*)this)->imgRequest::mRefCnt’
You need to log in before you can comment on or make changes to this bug.