Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add C API for Promise #1794

Closed
martijnthe opened this issue Apr 28, 2017 · 15 comments
Closed

Add C API for Promise #1794

martijnthe opened this issue Apr 28, 2017 · 15 comments

Comments

@martijnthe
Copy link
Contributor

Now we've got ES6 Promise support from within .js, I think it's a good time to add the C API as well :)

@martijnthe
Copy link
Contributor Author

cc @jiangzidong @jprestwo @grgustaf

@jiangzidong
Copy link
Contributor

sure, like jerry_create_promise and jerry_value_is_promise, right?

@martijnthe
Copy link
Contributor Author

Yeah and API to reject / fullfill, for example:
https://github.com/01org/zephyr.js/blob/master/src/zjs_promise.h

@jprestwo @grgustaf what else?

@jprestwo
Copy link

@martijnthe,
create, fulfill, and reject are all we use/need in ZJS. is_promise could be useful as well.

@martijnthe
Copy link
Contributor Author

@jprestwo, can you help me understand why you have the zjs_post_promise_func post argument in zjs_make_promise? Is this needed here too you think? Can't you use the native handle/pointer API for this?

@jprestwo
Copy link

@martijnthe, no that is not needed. I have a pending PR for ZJS that uses JerryScript promises and I took out that parameter:
intel/zephyr.js#1058

Of course this PR is irrelevant once JerryScript has C API's.

@jiangzidong
Copy link
Contributor

On second thought, I don't think the promise api in intel/zephyr.js#1058 is suitable for jerry C api. Because it saves the resolve/reject handler inside the native pointer of promise object, however, the ideal create_promise should return a promise object which has empty native pointer, so that users can set it depends on their own scenario.

I suggest the the signature should be

jerry_value_t jerry_create_promise (jerry_value_t *resolve_p, jerry_value_t *reject_p)

the resolve_p and reject_p are function object, and are OUT arguments, so that users get the handler, and should maintain them (including free them). When they want to resolve a promise, they just jerry_call_function(*resolve_p, ......). And we don't need the explicit api like fullfill/resolve/reject.

What do you think? @martijnthe @jprestwo

@martijnthe
Copy link
Contributor Author

@jiangzidong I'm not sure if I like that signature.
It is closest to how the JS Promise constructor works, but in reality I think it will be very cumbersome to use:

  1. Many async C APIs take a void *cb_data as parameter. With your proposal, you have to allocate a helper containing the 2 resolve and reject jerry values and pass a pointer to the helper as cb_data.
  2. You would have to use jerry_call_function with resolve and reject? I believe resolve and reject functions only take 0 or 1 argument and this isn't used by the resolve/reject function itself, so I think it makes sense to have a helper function for this, instead of having to deal with jerry_call_function directly.

See below for usage example.

// imaginary async OS API to get battery info
typedef struct {
    // ...
} batt_info_t;

typedef void (*batt_callback_t)(const batt_info_t *info, void *cb_data);

void os_get_battery_async(batt_callback_t cb, void *cb_data);

// option 1: -------------------------------------------------------------------

typedef struct {
    jerry_value_t resolve;
    jerry_value_t reject;
} promise_helper_t;

static void my_get_batt_info_cb (const batt_info_t *info, void *cb_data)
{
        promise_helper_t *helper = cb_data;

        // do stuff with *info and assign result + should_call_resolve:
        jerry_value_t result = ...;
        bool should_call_resolve = ...;

        jerry_value_t resolve_reject_result;
        resolve_reject_result = jerry_call_function(
                should_call_resolve ? helper->resolve : helper->reject,
                jerry_create_undefined() /* this */,
                &result, 1
        );
        // what to do with resolve_reject_result here? I think the Promise
        // implementation should be catching Errors, so do nothing?

        jerry_value_release(resolve_reject_result);
        jerry_value_release(helper->resolve);
        jerry_value_release(helper->reject);
        free(helper);
}

// 'getBattery' external function
// https://developer.mozilla.org/en-US/docs/Web/API/Navigator/getBattery
static jerry_value_t get_battery_impl (const jerry_value_t function_obj,
                                       const jerry_value_t this_val,
                                       const jerry_value_t args_p[],
                                       const jerry_length_t args_count)
{
        promise_helper_t *helper = malloc(sizeof(*helper));
        assert(helper);
        jerry_value_t promise = jerry_create_promise(&helper->resolve,
                                                     &helper->reject);
        os_get_battery_async(my_get_batt_info_cb, helper);
        return promise;
}

// option 2: -------------------------------------------------------------------

static void my_get_batt_info_cb (const batt_info_t *info, void *cb_data)
{
        jerry_value_t promise = (jerry_value_t)(uintptr_t) cb_data; 
        
        // do stuff with *info and assign result + should_call_resolve:
        jerry_value_t result = ...;
        bool should_call_resolve = ...;
        
        jerry_call_resolve_reject(promise, result, should_call_resolve);
        jerry_value_release(promise);
}

// 'getBattery' external function
// https://developer.mozilla.org/en-US/docs/Web/API/Navigator/getBattery
static jerry_value_t get_battery_impl (const jerry_value_t function_obj,
                                       const jerry_value_t this_val,
                                       const jerry_value_t args_p[],
                                       const jerry_length_t args_count)
{
        jerry_value_t promise = jerry_create_promise();
        os_get_battery_async(my_get_batt_info_cb,
                             (void *)(uintptr_t)jerry_value_acquire(promise));
        return promise;
}

@jiangzidong
Copy link
Contributor

jiangzidong commented May 1, 2017

Yes, I know what you mean, I can put these 2 function in the internal prop of the promise object (like the LIT_INTERNAL_MAGIC_STRING_NATIVE_POINTER, but not just in the native pointer).
And that will make the jerry_create_promise(), and jerry_promise_resolve/reject(...) be possible

@martijnthe
Copy link
Contributor Author

Yeah, that makes sense to me. Need to take care of breaking the "retain cycle" there: both resolve and reject function objects retain the promise object (through LIT_INTERNAL_MAGIC_STRING_PROMISE). Creating internal properties on the promise object that point to the functions would create a cycle, which would need to be broken at some point. Not sure if there's a good point in the life cycle to do this though...

Idea 3: stick with @jiangzidong 's proposal + add a utility to jerryx with helper functions to make it easier to use the low level API? Something like:

// jerry-core -----
jerry_value_t jerry_create_promise (jerry_value_t *resolve_p, jerry_value_t *reject_p);

// jerryx -----
typedef struct {
  jerry_value_t resolve;
  jerry_value_t reject;
} jerryx_promise_resolvers_t;

jerry_value_t jerryx_create_promise_and_resolvers (jerryx_promise_resolvers_t **resolvers_out);

typedef enum {
  JERRYX_RESOLVE,
  JERRYX_REJECT,
} jerryx_promise_resolver_type;

void jerryx_call_and_free_resolver (jerryx_promise_resolvers_t *resolvers, jerry_value_t value, jerryx_promise_resolver_type type);

@jiangzidong
Copy link
Contributor

Need to take care of breaking the "retain cycle" there: both resolve and reject function objects retain the promise object (through LIT_INTERNAL_MAGIC_STRING_PROMISE)

It should not be a problem. Though the promise object and the handler are marked by each other during gc, but that doesn't affect their ref count. As long as the promise object's ref == 0 and it cannot be visited by other visited object, both the promise and the handler will be GCed. It's like var a = {}; var b = {}; a.data = b ; b.data = a

@martijnthe
Copy link
Contributor Author

OK, if there's no problem there w.r.t. memory mgmt, I think it would be nice if the jerryscript.h API is easy to use directly. Something like this?

// returns promise object
jerry_value_t jerry_create_promise(void);

typedef enum {
  JERRY_PROMISE_RESOLVE, // spec talks about "fulfill" but I think "resolve" is more commonly used.
  JERRY_PROMISE_REJECT,
} jerry_promise_resolution_t; // not sure what to call this...

// Fulfils or rejects a promise
void jerry_resolve_promise(jerry_value_t promise, jerry_value_t value,
                           jerry_promise_resolution_t resolution);

// returns true if promise
bool jerry_value_is_promise(jerry_value_t value);

// returns resolve function object
jerry_value_t jerry_get_promise_resolve(jerry_value_t promise_obj);

// returns reject function object
jerry_value_t jerry_get_promise_reject(jerry_value_t promise_obj);

@jiangzidong jiangzidong self-assigned this May 2, 2017
@jiangzidong
Copy link
Contributor

see #1796

Why do we need the api like jerry_value_t jerry_get_promise_resolve(jerry_value_t promise_obj); ?

@martijnthe
Copy link
Contributor Author

Why do we need the api like jerry_value_t jerry_get_promise_resolve(jerry_value_t promise_obj); ?

I don't have a good concrete example at hand, but I feel that because the JS Promise API semantics hand the user 2 function objects (new Promise(function(resolve, reject) { ... })), there should be a way to get those functions in the native API as well.
Imaginary meta-examples: when the native binding wants to hand the resolve/reject function back to JS land, or to hand it to a piece of library code that takes a function object.

@jiangzidong
Copy link
Contributor

#1796 merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants