Closed Bug 374681 Opened 17 years ago Closed 6 years ago

JS engine warning: 260 GC roots remain after destroying the JSRuntime.

Categories

(Core :: JavaScript Engine, defect)

1.8 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: bc, Unassigned)

Details

Attachments

(2 files)

forked from bug 352064

the testcase in attachment 237646 [details] (zip file) in a 1.8.0 (but not 1.8.1) debug build with Java 6 from this morning.

shows lots of 

JS engine warning: leaking GC root '&member_descriptor->invoke_func_obj' at 03EA3B34

warnings with a summary

JS engine warning: 260 GC roots remain after destroying the JSRuntime.
                   These roots may point to freed memory. Objects reachable
                   through them have not been finalized.
I wonder if this is because the invoke_func_obj root is being added always, but only removed if the function ptr is non-null?
(In reply to comment #1)
> I wonder if this is because the invoke_func_obj root is being added always, but
> only removed if the function ptr is non-null?
 
That would do it... Can you take this one?

/be

Yeah.  Only question is, should I NOT root if the function is null, or should I remove the root regardless?  Which is the right direction to go?
Assignee: general → crowder
Alternative #1
Attachment #260617 - Flags: review?(brendan)
Keep whichever one of these you like better.
Attachment #260618 - Flags: review?(brendan)
Status: NEW → ASSIGNED
Wait, JS_GetFunctionObject will always return non-null after a successful JS_NewFunction return. So there's no point in the null-check guarding the call to JS_AddNamedRoot. Now the question is: why would the rooted pointer become null?

The first patch is good in any event, and it may be a fix. But how does the pointer become null (if it does)? If it doesn't, then there's no actual bug biting here -- just the latent bug of the null-check guarding the JS_RemoveRoot call.

/be
All of the memsets on the structure seem only to happen immediately following its allocation and I don't see any other obvious places where this pointer would be getting NULLed out....
If you are seeing

JS engine warning: leaking GC root '&member_descriptor->invoke_func_obj' at
03EA3B34

then indeed the JS_RemoveRoots are not happening, or are passing the wrong address, being called too late, whatever.

This needs debugging before patching. The first patch is a good change to the current code, but it won't fix the bug AFAICT. Please reproduce with it applied and debug why JS_RemoveRoot ain't working for these named roots.

/be
Attachment #260618 - Flags: review?(brendan) → review-
Comment on attachment 260617 [details] [diff] [review]
first proposal, remove root regardless

This can't hurt, could help but we think it won't. Try it?

/be
Attachment #260617 - Flags: review?(brendan) → review+
I can still see the same GC warning with the patch.
Well, I have tried now with 1.8, 1.8.0 and trunk nightly builds and cannot reproduce this bug on Mac OS X; it must be windows-only?  I will try on my windows box later.
Forgot to mention, here is Firefox 1.8.0 debug build on Solaris Nevada.
jsj_class.c: 1.24

Landed the reviewed patch here, but not closing out the bug as it seems unlikely to fix the problem.  Someone else who can reproduce this will have a better chance of fixing it than I did just auditing code, so I am releasing the bug into the wild.
Assignee: crowder → general
Status: ASSIGNED → NEW
Assignee: general → nobody
Old bug with test case using Java Applet, so probably no longer actionable, therefore resolving as INCOMPLETE.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: