Closed Bug 1588208 Opened 10 months ago Closed 10 months ago

DescribeRefCountedNode() gets called twice on CallbackTimeoutHandler and ScriptTimeoutHandler

Categories

(Core :: DOM: Core & HTML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox67 --- unaffected
firefox68 --- unaffected
firefox69 --- wontfix
firefox70 --- wontfix
firefox71 --- fixed

People

(Reporter: mccr8, Assigned: edgar)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

kmag sent me a log that caused an assertion in my cycle collector log parser. The error is on the second line here:

0x14fc80e2ea60 [rc=1] CallbackTimeoutHandler
0x14fc80e2ea60 [rc=1] TimeoutHandler

0x14fc9a14d800 mGlobal
0x14fc8a4a9c40 mFunction

The object 0x14fc80e2ea60 is getting reported twice, first as a CallbackTimeoutHandler, then as a TimeoutHandler.

This is a regression from bug 1558776, part 2. The problem is the traverse method for CallbackTimeoutHandler calls cb.DescribeRefCountedNode(), then calls the Traverse method for TimeoutHandler, which calls DescribeRefCountedNode a second time.

I think this is only a real problem if logging is on. (mVisitedRefCounted will end up being slightly wrong, but that's only for telemetry.)

I'm not sure what a good fix for this is. This is also an issue for ScriptTimeoutHandler.

It looks like you can actually delete the call to NS_CYCLE_COLLECTION_CLASSNAME(TimeoutHandler)::TraverseNative(s, cb), because it doesn't do anything, but the code will end up being fragile, because then if somebody innocently goes in and adds something to TimeoutHandler, the CC won't see it.

Summary: DescribeRefCountedNode() gets called twice on CallbackTimeoutHandler → DescribeRefCountedNode() gets called twice on CallbackTimeoutHandler and ScriptTimeoutHandler

(In reply to Andrew McCreight [:mccr8] from comment #0)

It looks like you can actually delete the call to NS_CYCLE_COLLECTION_CLASSNAME(TimeoutHandler)::TraverseNative(s, cb), because it doesn't do anything, but the code will end up being fragile, because then if somebody innocently goes in and adds something to TimeoutHandler, the CC won't see it.

Yes, TimeoutHandler doesn't do anything for CC, don't call NS_CYCLE_COLLECTION_CLASSNAME(TimeoutHandler)::TraverseNative(s, cb) is okay, and I don't have better soluton other than this. Thanks.

Assignee: nobody → echen
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ac01b45d14a1
Don't need to traverse to TimeoutHandler; r=mccr8
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.