379455 - Tracing of locked GC things should account for the lock count.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

The bug 377884 comment 12 gives a clear reason to call the tracer for locked GC thing the same number of times the thing is locked notwithstanding considerations in bug 375270 comment 37 and later.

Comment 1

[Igor Bukanov]

Attachment: fix v1

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 263648 [details] [diff] [review]
fix v1

>     /*
>-     * During GC marking JS_CALL_TRACER calls gcThingCallback once. But we need
>-     * to call the callback extra lhe->count - 1 times to report an accurate
>-     * reference count there.
>-     */
>-    if (IS_GC_MARKING_TRACER(trc) && (n = lhe->count - 1) != 0) {
>-        rt = trc->context->runtime;
>-        if (rt->gcThingCallback) {
>+     * Bug 379455: we run the tracer once, but to transfer the knowledge about
>+     * thing's lock count the tracer or gcThingCallback when the tracer is GC
>+     * marking phase we need to run them extra lhe->count - 1 times.

This runs on a bit. How about

     * Bug 379455: we called the tracer once, but to communicate the value of
     * thing's lock count to the tracer, or to gcThingCallback when the tracer
     * is the GC marking phase, we need to call an extra lhe->count - 1 times.

>+     */
>+    n = lhe->count - 1;
>+    if (n != 0) {
>+        if (IS_GC_MARKING_TRACER(trc)) {
>+            rt = trc->context->runtime;
>+            if (rt->gcThingCallback) {
>+                do {
>+                    rt->gcThingCallback(thing, flags,
>+                                        rt->gcThingCallbackClosure);
>+                } while (--n != 0);
>+            }

Can we get rid of gcThingCallback in due course? I mean, is there a dependent bug on file?

>+        } else {
>             do {
>-                rt->gcThingCallback(thing, flags, rt->gcThingCallbackClosure);
>+                JS_CALL_TRACER(trc, thing, traceKind, "locked object");
>             } while (--n != 0);
>         }
>     }
>     return JS_DHASH_NEXT;
> }
> 
> #define TRACE_JSVALS(trc, len, vec, name)                                     \

r=me with comment and followup bug attention.

/be

Comment 3

[Igor Bukanov]

(In reply to comment #2)
> Can we get rid of gcThingCallback in due course? I mean, is there a dependent
> bug on file?

See bug 379718.

Comment 4

[Igor Bukanov]

Attachment: fix v1b

Patch to commit with replaced comments. Thanks for the nice text!

Comment 5

[Igor Bukanov]

I commited the patch from comment 4 to the trunk:

Checking in jsgc.c;
/cvsroot/mozilla/js/src/jsgc.c,v  <--  jsgc.c
new revision: 3.217; previous revision: 3.216
done