search

laverdet / node-fibers

Fiber/coroutine support for v8 and node.
MIT License
3.56k stars 224 forks source link

initial work to get async resource working correctly #465

Closed znewsham closed 1 year ago

laverdet commented 1 year ago

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: https://github.com/laverdet/node-fibers/issues/365

znewsham commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

znewsham commented 1 year ago

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