From e3481d431b019bfdb12460b90077b63a427ecfbe Mon Sep 17 00:00:00 2001 From: Robert Fancsik Date: Fri, 9 Oct 2020 15:12:45 +0200 Subject: [PATCH] Revise the API ArrayBuffer related operations (#4284) - External ArrayBuffer construction with 0 length should be equivalent to `new ArrayBuffer(0)` - Internally allocated ArrayBuffers should be detachable - Externally allocated ArrayBuffers free callback should be called when underlying buffer is detached JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik frobert@inf.u-szeged.hu --- docs/02.API-REFERENCE.md | 6 +- jerry-core/api/jerry.c | 24 +++---- jerry-core/ecma/base/ecma-gc.c | 3 +- jerry-core/ecma/base/ecma-globals.h | 4 ++ .../ecma/operations/ecma-arraybuffer-object.c | 62 +++++++------------ .../ecma/operations/ecma-arraybuffer-object.h | 2 - tests/unit-core/test-arraybuffer.c | 51 ++++++++++++++- 7 files changed, 93 insertions(+), 59 deletions(-) diff --git a/docs/02.API-REFERENCE.md b/docs/02.API-REFERENCE.md index db26b7007..17baac2fd 100644 --- a/docs/02.API-REFERENCE.md +++ b/docs/02.API-REFERENCE.md @@ -9011,7 +9011,9 @@ jerry_get_arraybuffer_pointer (const jerry_value_t value); - `value` - Array Buffer object. - return value - pointer to the Array Buffer's data area. - - NULL if the `value` is not an Array Buffer object. + - NULL if the `value` is: + - not an ArrayBuffer object + - an external ArrayBuffer has been detached *New in version 2.0*. @@ -9096,6 +9098,8 @@ jerry_value_t jerry_detach_arraybuffer (const jerry_value_t value); ``` +*Note*: If the ArrayBuffer has been created with `jerry_create_arraybuffer_external` the optional free callback is called on a successful detach operation + - `value` - ArrayBuffer to be detached - return - null value if success diff --git a/jerry-core/api/jerry.c b/jerry-core/api/jerry.c index 9892aed16..c4a4bde9a 100644 --- a/jerry-core/api/jerry.c +++ b/jerry-core/api/jerry.c @@ -4204,7 +4204,7 @@ jerry_create_arraybuffer_external (const jerry_length_t size, /**< size of the b if (JERRY_UNLIKELY (size == 0 || buffer_p == NULL)) { - arraybuffer = ecma_arraybuffer_new_object_external (0, NULL, (ecma_object_native_free_callback_t) free_cb); + arraybuffer = ecma_arraybuffer_new_object (0); } else { @@ -4353,7 +4353,9 @@ jerry_get_arraybuffer_byte_length (const jerry_value_t value) /**< ArrayBuffer * * when accessing the pointer elements. * * @return pointer to the back-buffer of the ArrayBuffer. - * pointer is NULL if the parameter is not an ArrayBuffer + * pointer is NULL if: + * - the parameter is not an ArrayBuffer + * - an external ArrayBuffer has been detached */ uint8_t * jerry_get_arraybuffer_pointer (const jerry_value_t array_buffer) /**< Array Buffer to use */ @@ -4392,18 +4394,19 @@ jerry_is_arraybuffer_detachable (const jerry_value_t value) /**< ArrayBuffer */ if (ecma_is_arraybuffer (value)) { ecma_object_t *buffer_p = ecma_get_object_from_value (value); - return ecma_arraybuffer_is_detachable (buffer_p) ? ECMA_VALUE_TRUE : ECMA_VALUE_FALSE; + return ecma_arraybuffer_is_detached (buffer_p) ? ECMA_VALUE_FALSE : ECMA_VALUE_TRUE; } #else /* !ENABLED (JERRY_BUILTIN_TYPEDARRAY) */ JERRY_UNUSED (value); #endif /* ENABLED (JERRY_BUILTIN_TYPEDARRAY) */ - return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("Expects an ArrayBuffer"))); + return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("Expected an ArrayBuffer"))); } /* jerry_is_arraybuffer_detachable */ /** * Detach the underlying data block from ArrayBuffer and set its bytelength to 0. - * This operation requires the ArrayBuffer to be external that created by - * `jerry_create_arraybuffer_external`. + * + * Note: If the ArrayBuffer has been created with `jerry_create_arraybuffer_external` + * the optional free callback is called on a successful detach operation * * @return null value - if success * value marked with error flag - otherwise @@ -4417,17 +4420,16 @@ jerry_detach_arraybuffer (const jerry_value_t value) /**< ArrayBuffer */ if (ecma_is_arraybuffer (value)) { ecma_object_t *buffer_p = ecma_get_object_from_value (value); - bool detached = ecma_arraybuffer_detach (buffer_p); - if (!detached) + if (ecma_arraybuffer_detach (buffer_p)) { - return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("Expects a detachable ArrayBuffer."))); + return ECMA_VALUE_NULL; } - return ECMA_VALUE_NULL; + return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("ArrayBuffer has already been detached."))); } #else /* !ENABLED (JERRY_BUILTIN_TYPEDARRAY) */ JERRY_UNUSED (value); #endif /* ENABLED (JERRY_BUILTIN_TYPEDARRAY) */ - return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("Expects an ArrayBuffer"))); + return jerry_throw (ecma_raise_type_error (ECMA_ERR_MSG ("Expected an ArrayBuffer"))); } /* jerry_detach_arraybuffer */ /** diff --git a/jerry-core/ecma/base/ecma-gc.c b/jerry-core/ecma/base/ecma-gc.c index 16f7ca5df..3b6df8ca0 100644 --- a/jerry-core/ecma/base/ecma-gc.c +++ b/jerry-core/ecma/base/ecma-gc.c @@ -1384,11 +1384,10 @@ ecma_gc_free_object (ecma_object_t *object_p) /**< object to free */ /* Call external free callback if any. */ ecma_arraybuffer_external_info *array_p = (ecma_arraybuffer_external_info *) ext_object_p; - JERRY_ASSERT (array_p != NULL); if (array_p->free_cb != NULL) { - (array_p->free_cb) (array_p->buffer_p); + array_p->free_cb (array_p->buffer_p); } } else diff --git a/jerry-core/ecma/base/ecma-globals.h b/jerry-core/ecma/base/ecma-globals.h index 47f70cfa0..e5a127f82 100644 --- a/jerry-core/ecma/base/ecma-globals.h +++ b/jerry-core/ecma/base/ecma-globals.h @@ -1882,8 +1882,12 @@ typedef enum { ECMA_ARRAYBUFFER_INTERNAL_MEMORY = 0u, /* ArrayBuffer memory is handled internally. */ ECMA_ARRAYBUFFER_EXTERNAL_MEMORY = (1u << 0), /* ArrayBuffer created via jerry_create_arraybuffer_external. */ + ECMA_ARRAYBUFFER_DETACHED = (1u << 1), /* ArrayBuffer has been detached */ } ecma_arraybuffer_extra_flag_t; +/** + * Check whether the ArrayBuffer has external underlying buffer + */ #define ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY(object_p) \ ((((ecma_extended_object_t *) object_p)->u.class_prop.extra_info & ECMA_ARRAYBUFFER_EXTERNAL_MEMORY) != 0) diff --git a/jerry-core/ecma/operations/ecma-arraybuffer-object.c b/jerry-core/ecma/operations/ecma-arraybuffer-object.c index 58a1a1489..04a47d7ad 100644 --- a/jerry-core/ecma/operations/ecma-arraybuffer-object.c +++ b/jerry-core/ecma/operations/ecma-arraybuffer-object.c @@ -172,7 +172,7 @@ ecma_arraybuffer_get_length (ecma_object_t *object_p) /**< pointer to the ArrayB JERRY_ASSERT (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL)); ecma_extended_object_t *ext_object_p = (ecma_extended_object_t *) object_p; - return ext_object_p->u.class_prop.u.length; + return ecma_arraybuffer_is_detached (object_p) ? 0 : ext_object_p->u.class_prop.u.length; } /* ecma_arraybuffer_get_length */ /** @@ -190,12 +190,15 @@ ecma_arraybuffer_get_buffer (ecma_object_t *object_p) /**< pointer to the ArrayB if (ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY (ext_object_p)) { ecma_arraybuffer_external_info *array_p = (ecma_arraybuffer_external_info *) ext_object_p; + JERRY_ASSERT (!ecma_arraybuffer_is_detached (object_p) || array_p->buffer_p == NULL); return (lit_utf8_byte_t *) array_p->buffer_p; } - else + else if (ext_object_p->u.class_prop.extra_info & ECMA_ARRAYBUFFER_DETACHED) { - return (lit_utf8_byte_t *) (ext_object_p + 1); + return NULL; } + + return (lit_utf8_byte_t *) (ext_object_p + 1); } /* ecma_arraybuffer_get_buffer */ /** @@ -209,41 +212,9 @@ ecma_arraybuffer_is_detached (ecma_object_t *object_p) /**< pointer to the Array { JERRY_ASSERT (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL)); - ecma_extended_object_t *ext_object_p = (ecma_extended_object_t *) object_p; - - if (ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY (ext_object_p)) - { - ecma_arraybuffer_external_info *array_p = (ecma_arraybuffer_external_info *) ext_object_p; - /* in case the arraybuffer has been detached */ - return array_p->buffer_p == NULL; - } - - return false; + return (((ecma_extended_object_t *) object_p)->u.class_prop.extra_info & ECMA_ARRAYBUFFER_DETACHED) != 0; } /* ecma_arraybuffer_is_detached */ -/** - * Helper function: check if the target ArrayBuffer is detachable - * - * @return true - if value is an detachable ArrayBuffer object - * false - otherwise - */ -inline bool JERRY_ATTR_PURE JERRY_ATTR_ALWAYS_INLINE -ecma_arraybuffer_is_detachable (ecma_object_t *object_p) /**< pointer to the ArrayBuffer object */ -{ - JERRY_ASSERT (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL)); - - ecma_extended_object_t *ext_object_p = (ecma_extended_object_t *) object_p; - - if (ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY (ext_object_p)) - { - ecma_arraybuffer_external_info *array_p = (ecma_arraybuffer_external_info *) ext_object_p; - /* in case the arraybuffer has been detached */ - return array_p->buffer_p != NULL; - } - - return false; -} /* ecma_arraybuffer_is_detachable */ - /** * ArrayBuffer object detaching operation * @@ -257,16 +228,27 @@ ecma_arraybuffer_detach (ecma_object_t *object_p) /**< pointer to the ArrayBuffe { JERRY_ASSERT (ecma_object_class_is (object_p, LIT_MAGIC_STRING_ARRAY_BUFFER_UL)); - if (!ecma_arraybuffer_is_detachable (object_p)) + if (ecma_arraybuffer_is_detached (object_p)) { return false; } ecma_extended_object_t *ext_object_p = (ecma_extended_object_t *) object_p; + ext_object_p->u.class_prop.extra_info |= ECMA_ARRAYBUFFER_DETACHED; - ecma_arraybuffer_external_info *array_object_p = (ecma_arraybuffer_external_info *) ext_object_p; - array_object_p->buffer_p = NULL; - array_object_p->extended_object.u.class_prop.u.length = 0; + if (ECMA_ARRAYBUFFER_HAS_EXTERNAL_MEMORY (ext_object_p)) + { + ecma_arraybuffer_external_info *array_p = (ecma_arraybuffer_external_info *) ext_object_p; + + if (array_p->free_cb != NULL) + { + array_p->free_cb (array_p->buffer_p); + array_p->free_cb = NULL; + } + + ext_object_p->u.class_prop.u.length = 0; + array_p->buffer_p = NULL; + } return true; } /* ecma_arraybuffer_detach */ diff --git a/jerry-core/ecma/operations/ecma-arraybuffer-object.h b/jerry-core/ecma/operations/ecma-arraybuffer-object.h index 15bc8d284..d062542af 100644 --- a/jerry-core/ecma/operations/ecma-arraybuffer-object.h +++ b/jerry-core/ecma/operations/ecma-arraybuffer-object.h @@ -45,8 +45,6 @@ uint32_t JERRY_ATTR_PURE ecma_arraybuffer_get_length (ecma_object_t *obj_p); bool JERRY_ATTR_PURE ecma_arraybuffer_is_detached (ecma_object_t *obj_p); -bool JERRY_ATTR_PURE -ecma_arraybuffer_is_detachable (ecma_object_t *obj_p); bool ecma_arraybuffer_detach (ecma_object_t *obj_p); bool diff --git a/tests/unit-core/test-arraybuffer.c b/tests/unit-core/test-arraybuffer.c index b3735f0c2..c4be5c443 100644 --- a/tests/unit-core/test-arraybuffer.c +++ b/tests/unit-core/test-arraybuffer.c @@ -154,6 +154,7 @@ static void test_write_with_offset (uint8_t offset) /**< offset for buffer write } /* test_write_with_offset */ static bool callback_called = false; +static bool detach_free_callback_called = false; static void test_free_cb (void *buffer) /**< buffer to free (if needed) */ { @@ -161,6 +162,12 @@ static void test_free_cb (void *buffer) /**< buffer to free (if needed) */ callback_called = true; } /* test_free_cb */ +static void test_detach_free_cb (void *buffer) /**< buffer to free */ +{ + free (buffer); + detach_free_callback_called = true; +} /* test_detach_free_cb */ + int main (void) { @@ -256,6 +263,7 @@ main (void) jerry_value_t arraybuffer = jerry_create_arraybuffer_external (length, NULL, NULL); TEST_ASSERT (!jerry_value_is_error (arraybuffer)); TEST_ASSERT (jerry_value_is_arraybuffer (arraybuffer)); + TEST_ASSERT (jerry_is_arraybuffer_detachable (arraybuffer)); TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == length); uint8_t data[20]; @@ -364,7 +372,7 @@ main (void) jerry_release_value (buffer); } - /* Test ArrayBuffer detach */ + /* Test internal ArrayBuffer detach */ { const uint32_t length = 1; jerry_value_t arraybuffer = jerry_create_arraybuffer (length); @@ -374,10 +382,14 @@ main (void) jerry_value_t is_detachable = jerry_is_arraybuffer_detachable (arraybuffer); TEST_ASSERT (!jerry_value_is_error (is_detachable)); - TEST_ASSERT (!jerry_get_boolean_value (is_detachable)); + TEST_ASSERT (jerry_get_boolean_value (is_detachable)); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == length); + jerry_release_value (is_detachable); jerry_value_t res = jerry_detach_arraybuffer (arraybuffer); - TEST_ASSERT (jerry_value_is_error (res)); + TEST_ASSERT (!jerry_value_is_error (res)); + TEST_ASSERT (jerry_get_arraybuffer_pointer (arraybuffer) == NULL); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == 0); jerry_release_value (res); jerry_release_value (arraybuffer); @@ -395,10 +407,43 @@ main (void) jerry_value_t is_detachable = jerry_is_arraybuffer_detachable (arraybuffer); TEST_ASSERT (!jerry_value_is_error (is_detachable)); TEST_ASSERT (jerry_get_boolean_value (is_detachable)); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == length); jerry_release_value (is_detachable); jerry_value_t res = jerry_detach_arraybuffer (arraybuffer); TEST_ASSERT (!jerry_value_is_error (res)); + TEST_ASSERT (jerry_get_arraybuffer_pointer (arraybuffer) == NULL); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == 0); + + is_detachable = jerry_is_arraybuffer_detachable (arraybuffer); + TEST_ASSERT (!jerry_value_is_error (is_detachable)); + TEST_ASSERT (!jerry_get_boolean_value (is_detachable)); + jerry_release_value (is_detachable); + + jerry_release_value (res); + jerry_release_value (arraybuffer); + } + + /* Test external ArrayBuffer with callback detach */ + { + const uint32_t length = 8; + uint8_t *buf = (uint8_t *) malloc (length); + jerry_value_t arraybuffer = jerry_create_arraybuffer_external (length, buf, test_detach_free_cb); + TEST_ASSERT (!jerry_value_is_error (arraybuffer)); + TEST_ASSERT (jerry_value_is_arraybuffer (arraybuffer)); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == length); + + jerry_value_t is_detachable = jerry_is_arraybuffer_detachable (arraybuffer); + TEST_ASSERT (!jerry_value_is_error (is_detachable)); + TEST_ASSERT (jerry_get_boolean_value (is_detachable)); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == length); + jerry_release_value (is_detachable); + + jerry_value_t res = jerry_detach_arraybuffer (arraybuffer); + TEST_ASSERT (!jerry_value_is_error (res)); + TEST_ASSERT (jerry_get_arraybuffer_pointer (arraybuffer) == NULL); + TEST_ASSERT (jerry_get_arraybuffer_byte_length (arraybuffer) == 0); + TEST_ASSERT (detach_free_callback_called); is_detachable = jerry_is_arraybuffer_detachable (arraybuffer); TEST_ASSERT (!jerry_value_is_error (is_detachable));