Skip to content

Simplify exiting interpreter with exception#789

Merged
saghul merged 1 commit intomasterfrom
simplify-exception-exit
Jan 8, 2025
Merged

Simplify exiting interpreter with exception#789
saghul merged 1 commit intomasterfrom
simplify-exception-exit

Conversation

@saghul
Copy link
Copy Markdown
Contributor

@saghul saghul commented Jan 6, 2025

No description provided.

@saghul saghul force-pushed the simplify-exception-exit branch 2 times, most recently from 6a0624a to 196c0b4 Compare January 6, 2025 10:25
@saghul saghul marked this pull request as draft January 8, 2025 12:41
@saghul
Copy link
Copy Markdown
Contributor Author

saghul commented Jan 8, 2025

I just had an idea change things a bit more, will post an update.

- Avoid keeping the exception object around
- Avoid passing the responsibility of freeing the exeption object to the
  caller
@saghul saghul force-pushed the simplify-exception-exit branch from 196c0b4 to 1bf7525 Compare January 8, 2025 15:18
@saghul saghul requested a review from bnoordhuis January 8, 2025 15:18
@saghul saghul marked this pull request as ready for review January 8, 2025 15:18
@saghul
Copy link
Copy Markdown
Contributor Author

saghul commented Jan 8, 2025

@bnoordhuis PTAL, I think it's a cleaner implementation now, and more "backwars compatible" as in, code that calls js_std_loop won't leak any object in case of exception.

JS_FreeValue(ctx, ret);
JS_FreeContext(ctx);
js_std_free_handlers(rt);
JS_FreeContext(ctx);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I reversed the order here since I noticed that's how qjs.c does it.

@saghul saghul merged commit 97ea19d into master Jan 8, 2025
@saghul saghul deleted the simplify-exception-exit branch January 8, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants