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

initial work to get async resource working correctly #465

Closed
wants to merge 2 commits into from

Conversation

znewsham
Copy link

No description provided.

@laverdet
Copy link
Owner

I'm not sure this approach will work since fibers throws the entire stack model out the window and the nodejs async hooks API only provides push & pop operations which are stored directly on the environment:
https://github.com/nodejs/node/blob/e0c5b443af8b6e38406568244c81889ab2855f0f/src/async_wrap.cc#L255

There is additional information in this thread:
#365

@znewsham
Copy link
Author

Hi @laverdet thanks for the response - this is a very specific task (potentially not even relevant to the wider community) - I really only pushed it so the demo app that uses it can point here rather than cloning the repo.

in the effort to migrate our meteor apps off of fibers, there is value in decoupling the Meteor.EnvironmentVariable - which is implemented broadly as a Fiber.current._stuff = {}, which gets propagated around - to something that uses AsyncStorage. This way, we only need to care if we're in a fiber when we yield, rather than currently when calling Meteor.bindEnvironment (which doesn't yield, but does pull the current fiber's env vars)

The goal isn't really to change how fibers works, just to wrap the fiber in an async resource (in doing so, the AsyncStorage and executionAsyncId functions work as you'd expect).

In my (admittedly limited at this point) testing, everything seems to work as I'd expect.

Thanks for that thread - I'll give it a read!

@laverdet
Copy link
Owner

The setupAsyncHacks function that you commented out performs crucial bookkeeping updates which are not possible to replicate using the async_hooks API. nodejs assumes that an execution stack within a thread will fully unwind itself before executing code in another stack, which for most applications is a safe assumption. In the case of fibers we jump from stack to stack which is a violation of the assumed invariants of async_hooks.

@znewsham
Copy link
Author

Interesting, With it commented out everything did seem to work ok...I'll dig around some more, appreciate you taking a look!

@znewsham
Copy link
Author

I never intended to open this PR against the master repo :)

@znewsham znewsham closed this Oct 13, 2022
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.

2 participants