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 94734 ce72679ffb95dbccb8baaab799ffbe1447ffa258
parent 94733 810e8295b080a8680d8676ffecf78dac584ef76c
child 94735 966e23109ac29990428dccec6b269a6945bb5b48
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs742384
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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");