Bug 742384 - CDataFinalizer.dispose now returns a value. r=jorendorff
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Wed, 11 Apr 2012 10:46:19 +0200
changeset 91427 ce72679ffb95dbccb8baaab799ffbe1447ffa258
parent 91426 810e8295b080a8680d8676ffecf78dac584ef76c
child 91428 966e23109ac29990428dccec6b269a6945bb5b48
push id672
push usertim.taubert@gmx.de
push dateFri, 13 Apr 2012 10:22:59 +0000
treeherderfx-team@cb2e81306595 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs742384
milestone14.0a1
Bug 742384 - CDataFinalizer.dispose now returns a value. r=jorendorff
js/src/ctypes/CTypes.cpp
toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp
toolkit/components/ctypes/tests/jsctypes-test-finalizer.h
toolkit/components/ctypes/tests/unit/test_finalizer.js
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -283,17 +283,19 @@ namespace CDataFinalizer {
    * during finalization) or a CDataFinalizer JSObject. Always use NULL
    * if you are calling from a finalizer.
    */
   static void Cleanup(Private *p, JSObject *obj);
 
   /*
    * Perform the actual call to the finalizer code.
    */
-  static void CallFinalizer(Private *p, JSObject *ctypes);
+  static void CallFinalizer(CDataFinalizer::Private *p,
+                            int* errnoStatus,
+                            int32_t* lastErrorStatus);
 
   /*
    * Return the CType of a CDataFinalizer object, or NULL if the object
    * has been cleaned-up already.
    */
   static JSObject *GetCType(JSContext *cx, JSObject *obj);
 
   /*
@@ -6644,47 +6646,47 @@ CDataFinalizer::Construct(JSContext* cx,
 
 /*
  * Actually call the finalizer. Does not perform any cleanup on the object.
  *
  * Preconditions: |this| must be a |CDataFinalizer|, |p| must be non-null.
  * The function fails if |this| has gone through |Forget|/|Dispose|
  * or |Finalize|.
  *
- * In contexts that should update errno/winLastError, callers should provide
- * argument |ctypes|, which must point to global object ctypes. In other cases,
- * this argument should be NULL.
+ * This function does not alter the value of |errno|/|GetLastError|.
+ *
+ * If argument |errnoStatus| is non-NULL, it receives the value of |errno|
+ * immediately after the call. Under Windows, if argument |lastErrorStatus|
+ * is non-NULL, it receives the value of |GetLastError| immediately after the
+ * call. On other platforms, |lastErrorStatus| is ignored.
  */
 void
-CDataFinalizer::CallFinalizer(CDataFinalizer::Private *p, JSObject *ctypes)
+CDataFinalizer::CallFinalizer(CDataFinalizer::Private *p,
+                              int* errnoStatus,
+                              int32_t* lastErrorStatus)
 {
   int savedErrno = errno;
   errno = 0;
 #if defined(XP_WIN)
   int32_t savedLastError = GetLastError();
   SetLastError(0);
 #endif // defined(XP_WIN)
 
   ffi_call(&p->CIF, FFI_FN(p->code), p->rvalue, &p->cargs);
 
-  int errnoStatus = errno;
+  if (errnoStatus) {
+    *errnoStatus = errno;
+  }
   errno = savedErrno;
 #if defined(XP_WIN)
-  int32_t lastErrorStatus = GetLastError();
+  if (lastErrorStatus) {
+    *lastErrorStatus = GetLastError();
+  }
   SetLastError(savedLastError);
 #endif // defined(XP_WIN)
-
-  if (!ctypes) {
-    return;
-  }
-
-  JS_SetReservedSlot(ctypes, SLOT_ERRNO, INT_TO_JSVAL(errnoStatus));
-#if defined(XP_WIN)
-  JS_SetReservedSlot(ctypes, SLOT_LASTERROR, INT_TO_JSVAL(lastErrorStatus));
-#endif // defined(XP_WIN)
 }
 
 /*
  * Forget the value.
  *
  * Preconditions: |this| must be a |CDataFinalizer|.
  * The function fails if |this| has gone through |Forget|/|Dispose|
  * or |Finalize|.
@@ -6756,21 +6758,47 @@ CDataFinalizer::Methods::Dispose(JSConte
     return JS_FALSE;
   }
 
   jsval valType = JS_GetReservedSlot(obj, SLOT_DATAFINALIZER_VALTYPE);
   JS_ASSERT(!JSVAL_IS_PRIMITIVE(valType));
 
   JSObject *objCTypes = CType::GetGlobalCTypes(cx, JSVAL_TO_OBJECT(valType));
 
-  CDataFinalizer::CallFinalizer(p, objCTypes);
+  jsval valCodePtrType = JS_GetReservedSlot(obj, SLOT_DATAFINALIZER_CODETYPE);
+  JS_ASSERT(!JSVAL_IS_PRIMITIVE(valCodePtrType));
+  JSObject *objCodePtrType = JSVAL_TO_OBJECT(valCodePtrType);
+
+  JSObject *objCodeType = PointerType::GetBaseType(objCodePtrType);
+  JS_ASSERT(objCodeType);
+  JS_ASSERT(CType::GetTypeCode(objCodeType) == TYPE_function);
+
+  JSObject *resultType = FunctionType::GetFunctionInfo(objCodeType)->mReturnType;
+  jsval result = JSVAL_VOID;
+
+  int errnoStatus;
+#if defined(XP_WIN)
+  int32_t lastErrorStatus;
+  CDataFinalizer::CallFinalizer(p, &errnoStatus, &lastErrorStatus);
+#else
+  CDataFinalizer::CallFinalizer(p, &errnoStatus, NULL);
+#endif // defined(XP_WIN)
+
+  JS_SetReservedSlot(objCTypes, SLOT_ERRNO, INT_TO_JSVAL(errnoStatus));
+#if defined(XP_WIN)
+  JS_SetReservedSlot(objCTypes, SLOT_LASTERROR, INT_TO_JSVAL(lastErrorStatus));
+#endif // defined(XP_WIN)
+
+  if (ConvertToJS(cx, resultType, NULL, p->rvalue, false, true, &result)) {
+    CDataFinalizer::Cleanup(p, obj);
+    JS_SET_RVAL(cx, vp, result);
+    return true;
+  }
   CDataFinalizer::Cleanup(p, obj);
-
-  JS_SET_RVAL(cx, vp, JSVAL_VOID);
-  return JS_TRUE;
+  return false;
 }
 
 /*
  * Perform finalization.
  *
  * Preconditions: |this| must be the result of |CDataFinalizer|.
  * It may have gone through |Forget|/|Dispose|.
  *
@@ -6783,17 +6811,17 @@ CDataFinalizer::Finalize(JSFreeOp* fop, 
 {
   CDataFinalizer::Private *p = (CDataFinalizer::Private *)
     JS_GetPrivate(obj);
 
   if (!p) {
     return;
   }
 
-  CDataFinalizer::CallFinalizer(p, NULL);
+  CDataFinalizer::CallFinalizer(p, NULL, NULL);
   CDataFinalizer::Cleanup(p, NULL);
 }
 
 /*
  * Perform cleanup of a CDataFinalizer
  *
  * Release strong references, cleanup |Private|.
  *
--- a/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp
+++ b/toolkit/components/ctypes/tests/jsctypes-test-finalizer.cpp
@@ -72,16 +72,25 @@ size_t
 test_finalizer_rel_size_t_return_size_t(size_t i)
 {
   if (-- gFinalizerTestResources[i] < 0) {
     MOZ_NOT_REACHED("Assertion failed");
   }
   return i;
 }
 
+RECT
+test_finalizer_rel_size_t_return_struct_t(size_t i)
+{
+  if (-- gFinalizerTestResources[i] < 0) {
+    MOZ_NOT_REACHED("Assertion failed");
+  }
+  RECT result = { i, i, i, i };
+  return result;
+}
 
 bool
 test_finalizer_cmp_size_t(size_t a, size_t b)
 {
   return a==b;
 }
 
 // Resource type: int32_t
--- a/toolkit/components/ctypes/tests/jsctypes-test-finalizer.h
+++ b/toolkit/components/ctypes/tests/jsctypes-test-finalizer.h
@@ -3,24 +3,24 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "jsapi.h"
 
 #define EXPORT_CDECL(type)   NS_EXPORT type
 
 NS_EXTERN_C
 {
-
   EXPORT_CDECL(void) test_finalizer_start(size_t size);
   EXPORT_CDECL(void) test_finalizer_stop();
   EXPORT_CDECL(bool) test_finalizer_resource_is_acquired(size_t i);
 
   EXPORT_CDECL(size_t) test_finalizer_acq_size_t(size_t i);
   EXPORT_CDECL(void) test_finalizer_rel_size_t(size_t i);
   EXPORT_CDECL(size_t) test_finalizer_rel_size_t_return_size_t(size_t i);
+  EXPORT_CDECL(RECT) test_finalizer_rel_size_t_return_struct_t(size_t i);
   EXPORT_CDECL(bool) test_finalizer_cmp_size_t(size_t a, size_t b);
 
   EXPORT_CDECL(int32_t) test_finalizer_acq_int32_t(size_t i);
   EXPORT_CDECL(void) test_finalizer_rel_int32_t(int32_t i);
   EXPORT_CDECL(bool) test_finalizer_cmp_int32_t(int32_t a, int32_t b);
 
   EXPORT_CDECL(int64_t) test_finalizer_acq_int64_t(size_t i);
   EXPORT_CDECL(void) test_finalizer_rel_int64_t(int64_t i);
--- a/toolkit/components/ctypes/tests/unit/test_finalizer.js
+++ b/toolkit/components/ctypes/tests/unit/test_finalizer.js
@@ -8,16 +8,19 @@ function run_test()
                           ctypes.void_t,
                           ctypes.size_t);
   let stop = library.declare("test_finalizer_stop", ctypes.default_abi,
                          ctypes.void_t);
   let status = library.declare("test_finalizer_resource_is_acquired",
                            ctypes.default_abi,
                            ctypes.bool,
                            ctypes.size_t);
+  let released = function(value, witness) {
+    return witness == undefined;
+  };
 
   let samples = [];
   samples.push(
     {
       name: "size_t",
       acquire: library.declare("test_finalizer_acq_size_t",
                            ctypes.default_abi,
                            ctypes.size_t,
@@ -26,17 +29,18 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.size_t),
       compare: library.declare("test_finalizer_cmp_size_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.size_t,
                                ctypes.size_t),
-      status: status
+      status: status,
+      released: released
   });
   samples.push(
     {
       name: "size_t",
       acquire: library.declare("test_finalizer_acq_size_t",
                            ctypes.default_abi,
                            ctypes.size_t,
                            ctypes.size_t),
@@ -44,17 +48,18 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.size_t),
       compare: library.declare("test_finalizer_cmp_size_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.size_t,
                                ctypes.size_t),
-      status: status
+      status: status,
+      released: released
   });
   samples.push(
     {
       name: "int32_t",
       acquire: library.declare("test_finalizer_acq_int32_t",
                            ctypes.default_abi,
                            ctypes.int32_t,
                            ctypes.size_t),
@@ -62,17 +67,18 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.int32_t),
       compare: library.declare("test_finalizer_cmp_int32_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.int32_t,
                                ctypes.int32_t),
-      status: status
+      status: status,
+      released: released
     }
   );
   samples.push(
     {
       name: "int64_t",
       acquire: library.declare("test_finalizer_acq_int64_t",
                            ctypes.default_abi,
                            ctypes.int64_t,
@@ -81,17 +87,18 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.int64_t),
       compare: library.declare("test_finalizer_cmp_int64_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.int64_t,
                                ctypes.int64_t),
-      status: status
+      status: status,
+      released: released
     }
   );
   samples.push(
     {
       name: "ptr",
       acquire: library.declare("test_finalizer_acq_ptr_t",
                            ctypes.default_abi,
                            ctypes.PointerType(ctypes.void_t),
@@ -100,17 +107,18 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.PointerType(ctypes.void_t)),
       compare: library.declare("test_finalizer_cmp_ptr_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.void_t.ptr,
                                ctypes.void_t.ptr),
-      status: status
+      status: status,
+      released: released
     }
   );
   samples.push(
     {
       name: "string",
       acquire: library.declare("test_finalizer_acq_string_t",
                                ctypes.default_abi,
                                ctypes.char.ptr,
@@ -119,17 +127,18 @@ function run_test()
                                ctypes.default_abi,
                                ctypes.void_t,
                                ctypes.char.ptr),
       compare: library.declare("test_finalizer_cmp_string_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.char.ptr,
                                ctypes.char.ptr),
-      status: status
+      status: status,
+      released: released
     }
   );
   const rect_t = new ctypes.StructType("RECT",
                                        [{ top   : ctypes.int32_t },
                                         { left  : ctypes.int32_t },
                                         { bottom: ctypes.int32_t },
                                         { right : ctypes.int32_t }]);
   samples.push(
@@ -143,17 +152,18 @@ function run_test()
                                ctypes.default_abi,
                                ctypes.void_t,
                                rect_t),
       compare: library.declare("test_finalizer_cmp_struct_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                rect_t,
                                rect_t),
-      status: status
+      status: status,
+      released: released
     }
   );
   samples.push(
     {
       name: "size_t, release returns size_t",
       acquire: library.declare("test_finalizer_acq_size_t",
                            ctypes.default_abi,
                            ctypes.size_t,
@@ -162,50 +172,80 @@ function run_test()
                            ctypes.default_abi,
                            ctypes.size_t,
                            ctypes.size_t),
       compare: library.declare("test_finalizer_cmp_size_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.size_t,
                                ctypes.size_t),
-      status: status
+      status: status,
+      released: function(i, witness) {
+        return i == witness;
+      }
     }
   );
   samples.push(
     {
-      name: "null",
+      name: "size_t, release returns RECT",
+      acquire: library.declare("test_finalizer_acq_size_t",
+                           ctypes.default_abi,
+                           ctypes.size_t,
+                           ctypes.size_t),
+      release: library.declare("test_finalizer_rel_size_t_return_struct_t",
+                               ctypes.default_abi,
+                               rect_t,
+                               ctypes.size_t),
+      compare: library.declare("test_finalizer_cmp_size_t",
+                               ctypes.default_abi,
+                               ctypes.bool,
+                               ctypes.size_t,
+                               ctypes.size_t),
+      status: status,
+      released: function(i, witness) {
+        return witness.top == i
+          && witness.bottom == i
+          && witness.left == i
+          && witness.right == i;
+      }
+    }
+  );
+  samples.push(
+    {
+      name: "using null",
       acquire: library.declare("test_finalizer_acq_null_t",
                            ctypes.default_abi,
                            ctypes.PointerType(ctypes.void_t),
                            ctypes.size_t),
       release: library.declare("test_finalizer_rel_null_t",
                            ctypes.default_abi,
                            ctypes.void_t,
                            ctypes.PointerType(ctypes.void_t)),
       status: library.declare("test_finalizer_null_resource_is_acquired",
                              ctypes.default_abi,
                              ctypes.bool,
                              ctypes.size_t),
       compare: library.declare("test_finalizer_cmp_null_t",
                                ctypes.default_abi,
                                ctypes.bool,
                                ctypes.void_t.ptr,
-                               ctypes.void_t.ptr)
+                               ctypes.void_t.ptr),
+      released: released
     }
   );
 
   let tester = new ResourceTester(start, stop);
   samples.forEach(
     function(sample) {
       dump("Executing finalization test for data "+sample.name+"\n");
       tester.launch(TEST_SIZE, test_executing_finalizers, sample);
       tester.launch(TEST_SIZE, test_do_not_execute_finalizers_on_referenced_stuff, sample);
       tester.launch(TEST_SIZE, test_executing_dispose, sample);
       tester.launch(TEST_SIZE, test_executing_forget, sample);
+      tester.launch(TEST_SIZE, test_result_dispose, sample);
     }
   );
 
   /*
    * Following test deactivated: Cycle collection never takes place
    * (see bug 727371)
   tester.launch(TEST_SIZE, test_cycles, samples[0]);
    */
@@ -263,17 +303,37 @@ function test_executing_finalizers(size,
     ctypes.CDataFinalizer(tc.acquire(i), tc.release);
   }
   trigger_gc(); // This should trigger some finalizations, hopefully all
 
   // Check that _something_ has been finalized
   do_check_true(count_finalized(size, tc) > 0);
 }
 
+/**
+ * Check that
+ * - |dispose| returns the proper result
+ */
+function test_result_dispose(size, tc) {
+  dump("test_result_dispose " + tc.name + "\n");
+  let ref = [];
+  // Allocate |size| items with references
+  for (let i = 0; i < size; ++i) {
+    ref.push(ctypes.CDataFinalizer(tc.acquire(i), tc.release));
+  }
+  do_check_eq(count_finalized(size, tc), 0);
 
+  for (i = 0; i < size; ++i) {
+    let witness = ref[i].dispose();
+    ref[i] = null;
+    if (!tc.released(i, witness)) {
+      do_check_true(tc.released(i, witness));
+    }
+  }
+}
 /**
  * Check that
  * - |dispose| is executed properly
  * - finalizers are not executed after |dispose|
  */
 function test_executing_dispose(size, tc)
 {
   dump("test_executing_dispose\n");