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

Refactor jsg::V8StackScope into callback-based API #1560

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 20, 2024

Defining jsg::V8StackScope as a stack is problematic as it does not guarantee that the stack start will be set correctly in v8. This commit refactors the API to use a callback pattern instead.

jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
  // ...
});

See the primary change in jsg.h .. the rest is updating the usage sites.

As a follow-on PR, I also plan to change the Lock-taking API into a callback-style also.

@jasnell jasnell requested a review from kentonv January 20, 2024 01:37
@jasnell jasnell requested review from a team as code owners January 20, 2024 01:37
@jasnell jasnell force-pushed the jsnell/refactor-v8stackscope branch 2 times, most recently from e6478e1 to a26e9a0 Compare January 20, 2024 03:12
Defining `jsg::V8StackScope` as a stack is problematic as it does
not guarantee that the stack start will be set correctly in v8.
This commit refactors the API to use a callback pattern instead.

```
jsg::runInV8Stack([&](jsg::V8StackScope& stackScope) {
  // ...
});
```
src/workerd/jsg/jsg.h Outdated Show resolved Hide resolved
src/workerd/jsg/jsg.h Outdated Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/refactor-v8stackscope branch 2 times, most recently from 4331c36 to 183ec8f Compare January 22, 2024 16:07
src/workerd/io/worker.c++ Outdated Show resolved Hide resolved
src/workerd/io/worker.c++ Show resolved Hide resolved
src/workerd/io/worker.c++ Show resolved Hide resolved
src/workerd/server/server.c++ Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/refactor-v8stackscope branch from 183ec8f to 7a5188f Compare January 22, 2024 17:48
@jasnell jasnell merged commit 14dfa83 into main Jan 22, 2024
11 checks passed
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

Successfully merging this pull request may close these issues.

3 participants