Skip to content

fix bug: move jerry_make_api_unavailable into the end of jerry_cleanup #1277

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

Merged
merged 1 commit into from
Aug 24, 2016

Conversation

jiangzidong
Copy link
Contributor

In the old jerry_cleanup, it first jerry_make_api_unavailable , then call ecma_finalize ->ecma_gc_run

During gc, it may call

void
jerry_dispatch_object_free_callback (ecma_external_pointer_t freecb_p, /**< pointer to free callback handler */
                                     ecma_external_pointer_t native_p) /**< native handle, associated
                                                                        *   with freed object */
{
  jerry_make_api_unavailable ();

  ((jerry_object_free_callback_t) freecb_p) ((uintptr_t) native_p);

  jerry_make_api_available ();
} /* jerry_dispatch_object_free_callback */

which will jerry_make_api_available (); in the end

So in this way, we can't call jerry_init after jerry_cleanup, because

void
jerry_init (jerry_init_flag_t flags) /**< combination of Jerry flags */
{
  if (unlikely (jerry_api_available))
  {
    /* This function cannot be called twice unless jerry_cleanup is called. */
    JERRY_UNREACHABLE ();
  }
.....

In the patch, we move jerry_make_api_unavailable() to the end of jerry_cleanup to make sure the jerry_api_available==false after cleanup

@zherczeg
Copy link
Member

LGTM. Nice catch

@LaszloLango LaszloLango added bug Undesired behaviour critical Raises security concerns labels Aug 24, 2016
@LaszloLango LaszloLango added this to the Release 1.0 milestone Aug 24, 2016
@LaszloLango
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Undesired behaviour critical Raises security concerns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants