laverdet / node-fibers

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

Mac M1 - darwin arm64 support #454

Closed renanccastro closed 2 years ago

renanccastro commented 2 years ago

Hello!

I'm from Meteor and we have been trying to use Fibers with Mac M1 lately, but the CORO_PTHREAD implementation is producing deadlocks in some of our flows, like spawning any process inside a fiber. You can see more details in my answer here: https://github.com/meteor/meteor/discussions/11633#discussioncomment-1599596

In summary, seems like a pthread condition variable is being used even after the coroutine is deallocated. I haven't followed the analysis to a fix yet.

After some struggle with CORO_PTHREAD, I've changed the arm64 selector to use CORO_UCONTEXT passing the -D_XOPEN_SOURCE option which enables the usage of legacy ucontext library on mac.

It is working and in our analysis, it's pretty faster too!

I'm opening this PR in case you want to merge and release, which in this case we can also go back from our fork!

Thanks for your good work.

laverdet commented 2 years ago

Thanks for the contribution and the ongoing interest in node-fibers. I'd guess meteor is probably the largest remaining consumer of node-fibers so it's not the worst idea to continue maintenance in a fork. Like I wrote in the README earlier this year, this project is in a dead end trajectory with significant pushback from nodejs, v8, and the ecosystem as a whole. Let me know if you need any help with troubleshooting in the future!

GeoffreyBooth commented 2 years ago

@laverdet How would you feel about adding @renanccastro and other Meteor folks as additional maintainers of this repo? I feel like the overall community of node-fibers users would be best served by continued development happening here, where there’s the full history of issues and pull requests.

Regardless of whether development continues here or in a fork, do you mind sharing instructions on how to generate all the binaries that you create as part of publishing to npm? In https://github.com/laverdet/node-fibers/pull/384#issuecomment-417493601 you mentioned that it was a manual process for those. My project is dependent on the musl binary that works in Alpine.

laverdet commented 2 years ago

Sure, I wouldn't mind adding you guys on as contributors, and giving publishing permissions on npm.

The build process is literally just spinning up the various environments, running node-gyp build and copying the built binaries into bin/ and then publishing. There's a tool called node-pre-gyp which can automate a lot of this via travis-ci and other providers. I started the process to set that up a while ago but Windows support in Travis is pretty unreliable and they also don't support musl, so multiple build providers would be required. I already saw the writing on the wall for fibers so I ended up abandoning it. If continued support is on your radar I might recommend taking a look into it.

There's some commentary on this commit about continued support in nodejs and v8: https://github.com/laverdet/node-fibers/commit/8f2809869cc92c28c92880c4a38317ae3dbe654d

I'm pretty confident that blessed support for fibers could land in nodejs and v8 with relatively minor API changes to both. Honestly the problem is half technical and half political. I'm happy to mentor anyone who wants to go down that road.

renanccastro commented 2 years ago

Hi, @laverdet. Thanks for your time. We do plan to support fibers on Meteor, but also slowly migrate to async/await code. I don't think we can fight the flow here, unfortunately.

We plan to fix bugs, and probably support other architectures. If you can, please, add me and @filipenevola as contributors, and on npm: https://www.npmjs.com/~mdg, https://www.npmjs.com/~filipenevola and myself: https://www.npmjs.com/~renanccastro.

We can help whenever necessary.

Thanks again!

diavrank commented 2 years ago

Hi, is there any ETA for this fix?