Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions jerry-core/ecma/operations/ecma-jobqueue.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/* Copyright JS Foundation and other contributors, http://js.foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "ecma-globals.h"
#include "ecma-helpers.h"
#include "ecma-jobqueue.h"
#include "jerry-port.h"

#ifndef CONFIG_DISABLE_ES2015_PROMISE_BUILTIN

/** \addtogroup ecma ECMA
* @{
*
* \addtogroup ecmajobqueue ECMA Job Queue related routines
* @{
*/

/**
* The processor for PromiseReactionJob
*/
static ecma_value_t __attribute__ ((unused))
ecma_job_process_promise_reaction_job (void *job_p) /**< the job to be operated */
{
JERRY_UNUSED (job_p);
/** TODO: implement the function body */
return ecma_make_simple_value (ECMA_SIMPLE_VALUE_UNDEFINED);
} /* ecma_job_process_promise_reaction_job */

/**
* The processor for PromiseResolveThenableJob
*/
static ecma_value_t __attribute__ ((unused))
ecma_job_process_promise_thenable_job (void *job_p) /**< the job to be operated */
{
JERRY_UNUSED (job_p);
/** TODO: implement the function body */
return ecma_make_simple_value (ECMA_SIMPLE_VALUE_UNDEFINED);
} /* ecma_job_process_promise_thenable_job */

/**
* @}
* @}
*/
#endif /* !CONFIG_DISABLE_ES2015_PROMISE_BUILTIN */
33 changes: 33 additions & 0 deletions jerry-core/ecma/operations/ecma-jobqueue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* Copyright JS Foundation and other contributors, http://js.foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef ECMA_JOB_QUEUE_H
#define ECMA_JOB_QUEUE_H

#ifndef CONFIG_DISABLE_ES2015_PROMISE_BUILTIN

/** \addtogroup ecma ECMA
* @{
*
* \addtogroup ecmajobqueue ECMA Job Queue related routines
* @{
*/

/**
* @}
* @}
*/
#endif /* !CONFIG_DISABLE_ES2015_PROMISE_BUILTIN */
#endif /* !ECMA_JOB_QUEUE_H */
9 changes: 9 additions & 0 deletions jerry-core/jerry-port.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define JERRY_PORT_H

#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>

#ifdef __cplusplus
Expand Down Expand Up @@ -139,6 +140,14 @@ bool jerry_port_get_time_zone (jerry_time_zone_t *tz_p);
*/
double jerry_port_get_current_time (void);

#ifndef CONFIG_DISABLE_ES2015_PROMISE_BUILTIN

typedef uint32_t (*jerry_job_handler_t) (void *);

void jerry_port_jobqueue_enqueue (jerry_job_handler_t handler, void *job_p);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice, this way any binding that needs to execute something asynchronously (for example setTimeout()) can use this to schedule the callback. In other words, the JobQueue feature is also useful for other (non-ES2015 Promise dependent) use cases.

Because of this, I think it makes sense to make a separate CONFIG_DISABLE_ES2015_JOBQUEUE feature flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is too early to set the seperate CONFIG_DISABLE_ES2015_JOBQUEUE, the reasons are as follows:

  • for now, only promise will use jobqueue. Enabling only one of "JOBQUEUE" and "PROMISE" does not make much sense.
  • in the future, when we plan to support module (which also use jobqueue), we will enable jobqueue by #if !defined(CONFIG_DISABLE_PROMISE) || !defined(CONFIG_DISABLE_MODULE)
  • will setTimeout use the jobqueue? it is out of jerry-core's scope, because setTimeout is a external api. The binding code can directly use the jobqueue in the port no matter whether job queue is enabled in jerry-core. (That is another reason why I think the loop should not be in jerry-core)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, you're right. I've thought about this a bit more and I'm starting to doubt whether my comment here makes sense. The source of the ES6 job queue events/jobs is from things internal to JerryScript (for now only Promise), while for something like setTimeout(), the event source is the native/host system, which would put an event into the (existing) native event loop.


#endif /* !CONFIG_DISABLE_ES2015_PROMISE_BUILTIN */

/**
* @}
*/
Expand Down
1 change: 1 addition & 0 deletions jerry-core/profiles/es5.1.profile
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
CONFIG_DISABLE_ES2015_ARRAYBUFFER_BUILTIN
CONFIG_DISABLE_ES2015_BUILTIN
CONFIG_DISABLE_ES2015_PROMISE_BUILTIN
CONFIG_DISABLE_ES2015_TYPEDARRAY_BUILTIN
1 change: 1 addition & 0 deletions jerry-core/profiles/minimal.profile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CONFIG_DISABLE_DATE_BUILTIN
CONFIG_DISABLE_ERROR_BUILTINS
CONFIG_DISABLE_ES2015_ARRAYBUFFER_BUILTIN
CONFIG_DISABLE_ES2015_BUILTIN
CONFIG_DISABLE_ES2015_PROMISE_BUILTIN
CONFIG_DISABLE_ES2015_TYPEDARRAY_BUILTIN
CONFIG_DISABLE_JSON_BUILTIN
CONFIG_DISABLE_MATH_BUILTIN
Expand Down
141 changes: 141 additions & 0 deletions targets/default/jerry-port-default-jobqueue.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/* Copyright JS Foundation and other contributors, http://js.foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include "jerryscript.h"
#include "jerry-port.h"
#include "jerry-port-default.h"
#include "jmem.h"
#include "jrt.h"

#ifndef CONFIG_DISABLE_ES2015_PROMISE_BUILTIN

typedef struct jerry_port_queueitem_t jerry_port_queueitem_t;

/**
* Description of the queue item.
*/
struct jerry_port_queueitem_t
{
jerry_port_queueitem_t *next_p; /**< points to next item */
jerry_job_handler_t handler; /**< the handler for the job*/
void *job_p; /**< points to the job */
};

/**
* Description of a job queue (FIFO)
*/
typedef struct
{
jerry_port_queueitem_t *head_p; /**< points to the head item of the queue */
jerry_port_queueitem_t *tail_p; /**< points to the tail item of the queue*/
} jerry_port_jobqueue_t;

static jerry_port_jobqueue_t queue;

/**
* Initialize the job queue
*/
void jerry_port_jobqueue_init (void)
{
queue.head_p = NULL;
queue.tail_p = NULL;
} /* jerry_port_jobqueue_init */

/**
* Enqueue a job
*/
void jerry_port_jobqueue_enqueue (jerry_job_handler_t handler, /**< the handler for the job */
void *job_p) /**< the job */
{
jerry_port_queueitem_t *item_p = jmem_heap_alloc_block (sizeof (jerry_port_queueitem_t));
item_p->job_p = job_p;
item_p->handler = handler;

if (queue.head_p == NULL)
{
JERRY_ASSERT (queue.tail_p == NULL);

queue.head_p = item_p;
item_p->next_p = NULL;
queue.tail_p = item_p;

return;
}

JERRY_ASSERT (queue.tail_p != NULL);

queue.tail_p->next_p = item_p;
queue.tail_p = item_p;
} /* jerry_port_jobqueue_enqueue */

/**
* Dequeue and get the job.
* @return pointer to jerry_port_queueitem_t
* It should be freed with jmem_heap_free_block
*/
static jerry_port_queueitem_t *
jerry_port_jobqueue_dequeue (void)
{
if (queue.head_p == NULL)
{
JERRY_ASSERT (queue.tail_p == NULL);

return NULL;
}

JERRY_ASSERT (queue.tail_p != NULL);

jerry_port_queueitem_t *item_p = queue.head_p;
queue.head_p = queue.head_p->next_p;

return item_p;
} /* jerry_port_jobqueue_dequeue */

/**
* Start the jobqueue.
* @return jerry value
* If exception happens in the handler, stop the queue
* and return the exception.
* Otherwise, return undefined
*/
jerry_value_t
jerry_port_jobqueue_run (void)
{
jerry_value_t ret;

while (true)
{
jerry_port_queueitem_t *item_p = jerry_port_jobqueue_dequeue ();

if (item_p == NULL)
{
return jerry_create_undefined ();
}

void *job_p = item_p->job_p;
jerry_job_handler_t handler = item_p->handler;
jmem_heap_free_block (item_p, sizeof (jerry_port_queueitem_t));
ret = handler (job_p);

if (jerry_value_has_error_flag (ret))
{
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the NextJob section again (http://www.ecma-international.org/ecma-262/6.0/#sec-nextjob-result) I'm not sure whether the loop (recursion in the spec) should be stopped. It doesn't say that it returns after an error result. I guess you could argue that whether or not to return could fall under the implementation specific part.

That said, I think it's more useful to not return (keep looping) and instead call out to another port function to handle the error result, for example by using jerry_port_console() to print out the error message. I think something like that is what most people will do in actual projects.

I think it makes sense to expose a void jerry_port_handle_uncaught_error(jerry_value_t error) into jerry-port.h. Any user binding code that uses async callbacks can then also use the same interface to deal with uncaught errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more useful to not return (keep looping) and instead call out to another port function to handle the error result

In Node.js, the uncaught error will stop the loop

setTimeout(function(){console.log(x)}, 100); //error: no reference
setTimeout(function(){console.log("will not show this")}, 200); 

I know that the following code will run correctly

Promise.resolve(1).then(function(){console.log(x});
setTimeout(function(){console.log("WILL run here"), 100};

Because although function(){console.log(x} will throw error, it is not the result of NextJob, it is the step 7 of http://www.ecma-international.org/ecma-262/6.0/#sec-promisereactionjob

expose a handler into jerry-port.h

I doubt it. The caller of the handler is only the port-specific queue_run function, not the jerry-core.

The default port's queue is designed for main-unix.c as a simple demo, and it is only a simplest queue which cannot even handle real-async event. So in my design, the uncaught error will stop the queue, then developers can write their own error-process function in main-unix.c (usually print the error message). I wonder if it is necessary to increase the complexity of the default queue.

Copy link
Contributor

@martijnthe martijnthe Mar 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Node.js, the uncaught error will stop the loop

Ah, interesting. Chrome's behavior is different, it does actually print the "will not show this" log.

OK, let's just leave it as-is.

}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if no error flag, we also need to release the ret value. Will add in the updated patch soon


jerry_release_value (ret);
}
} /* jerry_port_jobqueue_run */

#endif /* !CONFIG_DISABLE_ES2015_PROMISE_BUILTIN */
6 changes: 6 additions & 0 deletions targets/default/jerry-port-default.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#ifndef JERRY_PORT_DEFAULT_H
#define JERRY_PORT_DEFAULT_H

#include "jerryscript.h"
#include "jerry-port.h"

#include <stdbool.h>
Expand All @@ -36,6 +37,11 @@ bool jerry_port_default_is_abort_on_fail (void);
jerry_log_level_t jerry_port_default_get_log_level (void);
void jerry_port_default_set_log_level (jerry_log_level_t level);

#ifndef CONFIG_DISABLE_ES2015_PROMISE_BUILTIN
void jerry_port_jobqueue_init (void);
jerry_value_t jerry_port_jobqueue_run (void);
#endif /* !CONFIG_DISABLE_ES2015_PROMISE_BUILTIN */

/**
* @}
*/
Expand Down