Bug 346450: The support for close hooks for generators is implemented through internal API and JSExtendedClass.close is removed as a premature feature. r=brendan sr=mrbkap

js/src/js.c

@@ -1870,7 +1870,7 @@ static JSExtendedClass split_global_class = {
     NULL, NULL, NULL, NULL, NULL, NULL,
     split_mark, NULL},
     NULL, split_outerObject, split_innerObject,
-    JSCLASS_NO_RESERVED_MEMBERS
+    NULL, NULL, NULL, NULL, NULL
 };
 
 JSObject *

js/src/jsapi.h

@@ -971,11 +971,11 @@ struct JSExtendedClass {
     JSEqualityOp        equality;
     JSObjectOp          outerObject;
     JSObjectOp          innerObject;
-    JSFinalizeOp        close;
-    jsword              reserved0;
-    jsword              reserved1;
-    jsword              reserved2;
-    jsword              reserved3;
+    void                (*reserved0)();
+    void                (*reserved1)();
+    void                (*reserved2)();
+    void                (*reserved3)();
+    void                (*reserved4)();
 };
 
 #define JSCLASS_HAS_PRIVATE             (1<<0)  /* objects have private slot */
@@ -1040,7 +1040,7 @@ struct JSExtendedClass {
 
 /* Initializer for unused members of statically initialized JSClass structs. */
 #define JSCLASS_NO_OPTIONAL_MEMBERS     0,0,0,0,0,0,0,0
-#define JSCLASS_NO_RESERVED_MEMBERS     0,0,0,0
+#define JSCLASS_NO_RESERVED_MEMBERS     0,0,0,0,0
 
 /* For detailed comments on these function pointer types, see jspubtd.h. */
 struct JSObjectOps {

js/src/jsgc.c

@@ -878,11 +878,13 @@ CloseIteratorStates(JSContext *cx)
 }
 
 JSBool
-js_AddObjectToCloseTable(JSContext *cx, JSObject *obj)
+js_RegisterGeneratorObject(JSContext *cx, JSObject *obj)
 {
     JSRuntime *rt;
     JSBool ok;
 
+    JS_ASSERT(OBJ_GET_CLASS(cx, obj) == &js_GeneratorClass);
+
     /*
      * Return early without doing anything if shutting down, to prevent a bad
      * close hook from ilooping the GC.  This could result in shutdown leaks,
@@ -890,17 +892,8 @@ js_AddObjectToCloseTable(JSContext *cx, JSObject *obj)
      */
     rt = cx->runtime;
     JS_ASSERT(!rt->gcRunning || rt->gcClosePhase);
-    if (rt->state == JSRTS_LANDING) {
-#ifdef DEBUG
-        fprintf(stderr,
-"JS API usage error: an extended class's close hook allocates another object\n"
-"also of extended class %s that has a close hook.  To prevent infinite loops\n"
-"when shutting down the JS garbage collector, this object will not be closed.\n"
-"This may result in a memory leak.\n",
-                OBJ_GET_CLASS(cx, obj)->name);
-#endif
+    if (rt->state == JSRTS_LANDING)
         return JS_TRUE;
-    }
 
     JS_LOCK_GC(rt);
     ok = AddToPtrTable(cx, &rt->gcCloseTable, &closeTableInfo, obj);
@@ -986,7 +979,6 @@ ExecuteCloseHooks(JSContext *cx, const JSObjectsToClose *toClose)
     JSStackFrame *fp;
     uint32 index, endIndex;
     JSObject *obj;
-    JSExtendedClass *xclasp;
     void **array;
 
     rt = cx->runtime;
@@ -1012,9 +1004,14 @@ ExecuteCloseHooks(JSContext *cx, const JSObjectsToClose *toClose)
          * new objects that have close hooks and reallocate the table.
          */
         obj = (JSObject *)rt->gcCloseTable.array[index];
-        xclasp = (JSExtendedClass *) LOCKED_OBJ_GET_CLASS(obj);
-        JS_ASSERT(xclasp->base.flags & JSCLASS_IS_EXTENDED);
-        xclasp->close(cx, obj);
+
+        /*
+         * Ignore errors until after we call the close method, then force
+         * prompt error reporting, since GC is infallible.
+         */
+        js_CloseGeneratorObject(cx, obj);
+        if (cx->throwing && !js_ReportUncaughtException(cx))
+            JS_ClearPendingException(cx);
     } while (++index != endIndex);
 
     rt->gcClosePhase = JS_FALSE;

js/src/jsgc.h

@@ -141,7 +141,7 @@ extern JSBool
 js_RegisterCloseableIterator(JSContext *cx, JSObject *obj);
 
 extern JSBool
-js_AddObjectToCloseTable(JSContext *cx, JSObject *obj);
+js_RegisterGeneratorObject(JSContext *cx, JSObject *obj);
 
 /*
  * The private JSGCThing struct, which describes a gcFreeList element.

js/src/jsiter.c

@@ -607,25 +607,19 @@ typedef struct JSGenerator {
     jsval               stack[1];
 } JSGenerator;
 
-static void
-generator_closehook(JSContext *cx, JSObject *obj)
+JSBool
+js_CloseGeneratorObject(JSContext *cx, JSObject *obj)
 {
-    JSGenerator *gen;
     jsval fval, rval;
     const jsid id = ATOM_TO_JSID(cx->runtime->atomState.closeAtom);
 
-    gen = (JSGenerator *) JS_GetPrivate(cx, obj);
-    if (!gen)
-        return;
+    JS_ASSERT(OBJ_GET_CLASS(cx, obj) == &js_GeneratorClass);
+    JS_ASSERT(JS_GetPrivate(cx, obj));
 
-   /*
-    * Ignore errors until after we call the close method, then force prompt
-    * error reporting, since GC is infallible.
-    */
-    if (JS_GetMethodById(cx, obj, id, &obj, &fval))
-        js_InternalCall(cx, obj, fval, 0, NULL, &rval);
-    if (cx->throwing && !js_ReportUncaughtException(cx))
-        JS_ClearPendingException(cx);
+    if (!JS_GetMethodById(cx, obj, id, &obj, &fval))
+        return JS_FALSE;
+
+    return js_InternalCall(cx, obj, fval, 0, NULL, &rval);
 }
 
 static void
@@ -657,16 +651,14 @@ generator_mark(JSContext *cx, JSObject *obj, void *arg)
     return 0;
 }
 
-JSExtendedClass js_GeneratorClass = {
-  { js_Generator_str,
+JSClass js_GeneratorClass = {
+    js_Generator_str,
     JSCLASS_HAS_PRIVATE | JSCLASS_IS_ANONYMOUS | JSCLASS_IS_EXTENDED |
     JSCLASS_HAS_CACHED_PROTO(JSProto_Generator),
     JS_PropertyStub,  JS_PropertyStub, JS_PropertyStub, JS_PropertyStub,
     JS_EnumerateStub, JS_ResolveStub,  JS_ConvertStub,  generator_finalize,
     NULL,             NULL,            NULL,            NULL,
-    NULL,             NULL,            generator_mark,  NULL },
-    NULL,             NULL,            NULL,            generator_closehook,
-    JSCLASS_NO_RESERVED_MEMBERS
+    NULL,             NULL,            generator_mark,  NULL
 };
 
 JSObject *
@@ -677,7 +669,8 @@ js_NewGenerator(JSContext *cx, JSStackFrame *fp)
     JSGenerator *gen;
     jsval *newsp;
 
-    obj = js_NewObject(cx, &js_GeneratorClass.base, NULL, NULL);
+    /* After the following return, failing control flow must goto bad. */
+    obj = js_NewObject(cx, &js_GeneratorClass, NULL, NULL);
     if (!obj)
         return NULL;
 
@@ -691,10 +684,8 @@ js_NewGenerator(JSContext *cx, JSStackFrame *fp)
     /* Allocate obj's private data struct. */
     gen = (JSGenerator *)
           JS_malloc(cx, sizeof(JSGenerator) + (nslots - 1) * sizeof(jsval));
-    if (!gen || !JS_SetPrivate(cx, obj, gen)) {
-        JS_free(cx, gen);
-        return NULL;
-    }
+    if (!gen)
+        goto bad;
 
     /* Copy call-invariant object and function references. */
     gen->frame.callobj = fp->callobj;
@@ -746,7 +737,24 @@ js_NewGenerator(JSContext *cx, JSStackFrame *fp)
 
     /* Note that gen is newborn. */
     gen->state = JSGEN_NEWBORN;
+
+    if (!JS_SetPrivate(cx, obj, gen)) {
+        JS_free(cx, gen);
+        goto bad;
+    }
+
+    if (!js_RegisterGeneratorObject(cx, obj)) {
+        /*
+         * Do not free gen here, as the finalizer will do that since we
+         * called JS_SetPrivate.
+         */
+        goto bad;
+    }
     return obj;
+
+  bad:
+    cx->newborn[GCX_OBJECT] = NULL;
+    return NULL;
 }
 
 static JSBool
@@ -760,7 +768,7 @@ generator_send(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
     JSBool ok;
     jsval junk;
 
-    if (!JS_InstanceOf(cx, obj, &js_GeneratorClass.base, argv))
+    if (!JS_InstanceOf(cx, obj, &js_GeneratorClass, argv))
         return JS_FALSE;
 
     gen = (JSGenerator *)JS_GetPrivate(cx, obj);
@@ -890,7 +898,7 @@ js_InitIteratorClasses(JSContext *cx, JSObject *obj)
         return NULL;
     proto->slots[JSSLOT_ITER_STATE] = JSVAL_NULL;
 
-    if (!JS_InitClass(cx, obj, NULL, &js_GeneratorClass.base, NULL, 0,
+    if (!JS_InitClass(cx, obj, NULL, &js_GeneratorClass, NULL, 0,
                       NULL, generator_methods, NULL, NULL)) {
         return NULL;
     }

js/src/jsiter.h

@@ -95,7 +95,10 @@ js_ThrowStopIteration(JSContext *cx, JSObject *obj);
 extern JSObject *
 js_NewGenerator(JSContext *cx, JSStackFrame *fp);
 
-extern JSExtendedClass  js_GeneratorClass;
+extern JSBool
+js_CloseGeneratorObject(JSContext *cx, JSObject *obj);
+
+extern JSClass          js_GeneratorClass;
 extern JSClass          js_IteratorClass;
 extern JSClass          js_StopIterationClass;
 extern JSClass          js_GeneratorExitClass;

js/src/jsobj.c

@@ -2398,13 +2398,6 @@ js_NewObject(JSContext *cx, JSClass *clasp, JSObject *proto, JSObject *parent)
     /* Store newslots after initializing all of 'em, just in case. */
     obj->slots = newslots;
 
-    /* If obj needs to be closed before being finalized, remember it. */
-    if ((clasp->flags & JSCLASS_IS_EXTENDED) &&
-        ((JSExtendedClass *)clasp)->close &&
-        !js_AddObjectToCloseTable(cx, obj)) {
-        goto bad;
-    }
-
     if (cx->runtime->objectHook) {
         JS_KEEP_ATOMS(cx->runtime);
         cx->runtime->objectHook(cx, obj, JS_TRUE, cx->runtime->objectHookData);

js/src/jspubtd.h

@@ -257,9 +257,6 @@ typedef JSBool
  * Finalize obj, which the garbage collector has determined to be unreachable
  * from other live objects or from GC roots.  Obviously, finalizers must never
  * store a reference to obj.
- *
- * This is also the type of the JSExtendedClass.close hook, which is stubbed
- * with NULL if not needed.
  */
 typedef void
 (* JS_DLL_CALLBACK JSFinalizeOp)(JSContext *cx, JSObject *obj);

js/src/jsxml.c

@@ -248,7 +248,7 @@ JS_FRIEND_DATA(JSExtendedClass) js_NamespaceClass = {
     NULL,              NULL,              NULL,              NULL,
     NULL,              NULL,              namespace_mark,    NULL },
     namespace_equality,NULL,              NULL,              NULL,
-    JSCLASS_NO_RESERVED_MEMBERS
+    NULL,              NULL,              NULL,              NULL
 };
 
 #define NAMESPACE_ATTRS                                                       \
@@ -451,7 +451,7 @@ JS_FRIEND_DATA(JSExtendedClass) js_QNameClass = {
     NULL,              NULL,              NULL,              NULL,
     NULL,              NULL,              qname_mark,        NULL },
     qname_equality,    NULL,              NULL,              NULL,
-    JSCLASS_NO_RESERVED_MEMBERS
+    NULL,              NULL,              NULL,              NULL
 };
 
 /*

js/src/xpconnect/src/xpcwrappednativejsops.cpp

@@ -891,8 +891,7 @@ JSExtendedClass XPC_WN_NoHelper_JSClass = {
     XPC_WN_Equality,
     XPC_WN_OuterObject,
     XPC_WN_InnerObject,
-    nsnull,
-    JSCLASS_NO_RESERVED_MEMBERS
+    nsnull,nsnull,nsnull,nsnull,nsnull
 };