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

feat: async context #24402

Merged
merged 1 commit into from
Aug 2, 2024
Merged

feat: async context #24402

merged 1 commit into from
Aug 2, 2024

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Jul 2, 2024

We are switching to ContinuationPreservedEmbedderData. This allows adding async context tracking to the various async operations that deno provides.

Fixes: #7010
Fixes: #22886
Fixes: #24368

@devsnek devsnek added the ci-draft Run the CI on draft PRs. label Jul 2, 2024
@devsnek devsnek force-pushed the async-context branch 3 times, most recently from 66783c5 to 5889a93 Compare July 2, 2024 23:59
tests/unit_node/async_hooks_test.ts Show resolved Hide resolved
ext/web/02_timers.js Outdated Show resolved Hide resolved
ext/node/polyfills/async_hooks.ts Show resolved Hide resolved
@devsnek devsnek force-pushed the async-context branch 4 times, most recently from 67d1585 to 8ba06ce Compare July 9, 2024 18:31
@devsnek devsnek marked this pull request as ready for review July 31, 2024 23:23
@devsnek devsnek added node compat node API polyfill Related to various "node:*" modules APIs and removed ci-draft Run the CI on draft PRs. labels Jul 31, 2024
Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Nice work! Just left one question

runtime/js/01_async_context.js Show resolved Hide resolved
Comment on lines +91 to +101
const unboundCallback = callback;
const asyncContext = getAsyncContext();
callback = () => {
const oldContext = getAsyncContext(asyncContext);
try {
setAsyncContext(asyncContext);
ReflectApply(unboundCallback, globalThis, args);
} finally {
setAsyncContext(oldContext);
}
};
Copy link
Member

Choose a reason for hiding this comment

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

Great that it will now work with timers!

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@devsnek devsnek merged commit 3a1a1cc into main Aug 2, 2024
17 checks passed
@devsnek devsnek deleted the async-context branch August 2, 2024 15:14
devsnek added a commit that referenced this pull request Aug 2, 2024
devsnek added a commit that referenced this pull request Aug 2, 2024
Reverts #24402

deno_web can't depend on code in runtime
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node API polyfill Related to various "node:*" modules APIs node compat
Projects
None yet
3 participants