Promise Continuation

[bkimman]

I am now looking at promises and have provided the PromiseContinuationCallback for the runtime.

In the callback, I am adding the 'task' provided to a 'promises_to_keep' queue and then when 'control' comes back to my code .. I am processing this queue -calling the 'task' function with JsUndefinedValue as the single argument.

All this works fine.

In the neospehere code base I noticed that the return value of the task is checked .. if its an object which has a 'then' property, it is calling another 'native' function which seems to do nothing but then I cannot be sure I understood everything there.

So the question is whether a check should be made for a 'then' property on the return value of the 'task' and then should something be done with that.

In the documentation regarding promises I do see reference to 'thenables' but have not understood that fully as yet .. is my question linked to this?

Thanks

K

[rhuanjl]

I assume the bit of neoSphere you're looking at is here: https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/shared/jsal.c#L340-L366

This is distinct from processing the promise queue, it's specifically looking at Rejected Promises. JsSetHostPromiseRejectionTracker allows you to specify a function to be called whether a promise is rejected and does not already have a .then() or .catch() assigned to handle that rejection.

However it's possible for the JS to attach a .then() after the rejectionTracker function is called - this triggers a second call of the rejectionTracker function with a "handled = true" flag.

In neoSphere a rejected promise that remains unhandled is treated as an unrecoverable error; to do this, the call to rejectionTracker function with "handled = false" adds the rejected Promise to a queue with "handled=false" IF the rejectionTracker is called again with "handled = true" it finds the promise in the queue and sets "handled=true" for it. See implementation of rejectionTracker here: https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/shared/jsal.c#L3136-L3159

Once all pending code and pending promises continuations have run, you reach the piece you noticed (line 340) and the queue of rejected promises is reviewed and if any are still unhandled neoSphere displays an error message and exits.

NOTE: this behaviour is not required by the javaScript spec - the specification requires the engine to inform its host about unhandled promise rejections but the treatment of them is implementation defined.

As an unhandled rejected promise normally means some asynchronous work which the programme depended on has failed displaying an error message can be very helpful for debugging - as potentially you don't know what went wrong otherwise.

[bkimman]

Thanks .. the part I was referring to was not related to the rejection tracking but normal processing.

I have put at the bottom the sequence of calls in neosphere. Briefly what happens is : a) when a task is received at the PromiseContinuationCallback a new 'script' is added to the internal dispatcher queue b) this script gets executed when the dispatcher gets to it .. the 'task' provided in the continue callback is called c) the return value of this task is checked - if its an object which has a 'then' property, then it causes a new native function to be called which does nothing

However it's possible for the JS to attach a .then() after the rejectionTracker function is called

I guess this would also apply to promises that are successful ... so the 'call' to do nothing if there is a .then property is perhaps giving chakra a chance to check if some function got added and if so it would make another call to the PromiseContinuationCallback .. and that would get scheduled as above. Without this, the added .then would be lost and ignored.

Does that sound correct? If yes, its a very elegant solution to let chakra do its work correctly.

K

Apologies but the formatting below got a little garbled and I cant seem to fix it.

1. Set up the callback for promise continuation https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/main.c#L655-L655 jsal_on_enqueue_job(on_enqueue_js_job);

https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/shared/jsal.c#L391-L395 that sets s_job_callback in jsal.c

2. jsal initialisation .. set the promise continuation callback https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/shared/jsal.c#L204-L204 JsSetPromiseContinuationCallback(on_resolve_reject_promise, NULL);

3. promise continuation callback : on_resolve_reject_promise

https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/shared/jsal.c#L3208-L3235

puts the 'task' on the stack
call s_job_callback -> this is on_enqueue_js_job

4. new function is created with the 'task' on the top of the task and added to the jobs queue in dispatch https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/main.c#L538-L545

static void
    on_enqueue_js_job(void)
    {
        script_t* script;

        script = script_new_function(0);
        dispatch_defer(script, 0, JOB_ON_TICK, true);
    }

https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/script.c#L146-L174

        script_new_function(int stack_index) 
        script_new_method(int stack_index)

5. dispatcher schedules the script to be run https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/dispatch.c#L200-L266 dispatch_run script_run(job->script, false); // invalidates job pointer

6. the script is run https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/script.c#L196-L240

// line 226 to 231

jsal_call_method(0); // runs the script .. check the return value
    if (jsal_is_object(-1) && jsal_has_prop_key(-1, s_key_then)) {
        jsal_get_prop_string(-1, "then");
        jsal_pull(-2);
        // create a new native function which is called immediately
        jsal_push_new_function(js_onScriptFinished, "", 0, false, (intptr_t)script);
        jsal_call_method(1);
        jsal_pop(1);
    }

7. This function does nothing
https://github.com/spheredev/neosphere/blob/a88fb097668e66f23b330703d9ad74d3c959f912/src/neosphere/script.c#L242-L251

static bool
js_onScriptFinished(int num_args, bool is_ctor, intptr_t magic)
{
    script_t* script;

    script = (script_t*)magic;
    script->in_use = false;
    script_unref(script);
    return false;
}

[rhuanjl]

Ah, sorry I'd searched neosphere source for "then" and picked the wrong occurrence.

That bit of logic goes beyond simply implementing the JS spec. It relates to a neosphere ability to set "update" scripts, which are scripts to be called every frame. This is detecting if an update script returns a promise (or promise like object) - if it does it's set to being "in-use" and not called on subsequent frames until the promise resolves.

[fatcerberus]

^^^ what he said. I don't blame you for getting confused by this though, the stack manipulation is often tricky to follow.

It's not actually calling js_onScriptFinished immediately; it's checking if the script returned a promise and if it did, it does scriptPromise.then(js_onScriptFinished) (the JSAL stack for the call is .then - promise - js_onScriptFinished, i.e. method - this - args). In this way the script remains marked as "in use" until the promise resolves; if a recurring script takes more than one frame to finish, neoSphere won't re-call it until the previous call finishes, but other scripts won't be blocked.

As far as the JS spec is concerned the only thing you need to do is to queue the function to be called on the next event loop tick. That's all. The reason the engine doesn't just call the function itself is because the spec requires promise continuations to be called outside the normal flow of JS execution, but the exact queuing mechanism is left up to the host.

tl;dr: The buck stops with on_enqueue_js_job.

[bkimman]

Thanks very much .. now its clear.