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

rework luv work vm storage into userdata #757

Merged
merged 1 commit into from
Feb 1, 2025
Merged

Conversation

truemedian
Copy link
Member

@truemedian truemedian commented Jan 29, 2025

Moves work vm storage into a garbage collected userdata. Allowing for each lua state to manage its own storage. This completely removes the need for a global array of work vms.

Better than #755 because it does not introduce an extra interface that integrators have to deal with.

Essentially an expansion of #756 by using the userdata object to store the entire vm array.

In particular, this should mimic pre loop __gc behavior of multiple lua states having access to a pool of work vms and that pool being cleaned up automatically everywhere; the existing loop __gc does not handle this case well, particularly when one state is closed first (the other state may then attempt to use closed lua states).

Fixes #754

@clason
Copy link

clason commented Jan 29, 2025

Verified

This commit was signed with the committer’s verified signature.
moves work vm storage into a garbage collected userdata.
this allows for each lua state to manage its own storage.

this removes the need for a global array of work vms.
@truemedian
Copy link
Member Author

thread - getpriority, setpriority seems to be flaky with returning EINPROGRESS, not entirely sure why it's failing; libuv is however returning errno instead of the return value, so all we know is that it's failing (the errno value is not set by pthreads and is hence garbage).

@squeek502
Copy link
Member

thread - getpriority, setpriority seems to be flaky with returning EINPROGRESS

#730

Copy link
Contributor

@Bilal2453 Bilal2453 left a comment

Choose a reason for hiding this comment

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

I like this more than the previous globals approach frankly.

@squeek502 squeek502 requested a review from zhaozg January 31, 2025 05:15
@zhaozg
Copy link
Member

zhaozg commented Feb 1, 2025

Looks great to me

@zhaozg zhaozg merged commit 657a08f into luvit:master Feb 1, 2025
14 checks passed
@clason
Copy link

clason commented Feb 1, 2025

With this merged, luv now passes all Neovim tests: neovim/neovim#32174

So if you can make a new fixup release, I'll be able to include it. Thanks!

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.

loop __gc is not set when luv_set_loop is used.
5 participants