Skip to content

Conversation

@tlxpro
Copy link

@tlxpro tlxpro commented Mar 1, 2025

I was experimenting with https://github.com/HiPhish/neotest-busted and I had issues with data being corrupted when read after multiple commands to busted completed concurrently. Seems like the issue is with multiple calls to uv_fs_write. According to libuv/libuv#3820 (comment), the responsabilty to control concurrent access to same file must lie with the user.

NVIM v0.11.0-nightly+1fb606b
Build type: Release
LuaJIT 2.1.1713773202
OSX

rcarriga added a commit that referenced this pull request Apr 18, 2025
@rcarriga
Copy link
Collaborator

I'm not sure if this actually fixes the issue as you're just sending the write to a new coroutine, you could still have several concurrent writes. Your test is failing for me locally.

I've just pushed a change to prevent concurrent writes, can you check with the latest master if you still the issue?

@tlxpro
Copy link
Author

tlxpro commented Apr 18, 2025

With the new coroutine the async context is no longer managed by libuv as it calls the sync version of fs_write.

local ok, write_err = vim.uv.fs_write(output_fd, data, nil)
assert(ok, write_err)

The test fail on latest master. But doesn't If I use fs_write sync.

rcarriga added a commit that referenced this pull request Apr 19, 2025
@rcarriga
Copy link
Collaborator

Ah sorry yes, I've changed it now so it will await the write. I still don't get the test passing with the sync version you have above or with the nio version I've implemented unfortunately

@tlxpro
Copy link
Author

tlxpro commented Apr 19, 2025

It might be due to how my system open files, I have a not so recent version of OSX. 10.15.

It didn't change anything for me when I tried this, but this might work for you.

local open_err, output_fd = nio.uv.fs_open(output_path, "w", tonumber("4010666", 8)) -- with O_SYNC
assert(not open_err, open_err)

@tlxpro
Copy link
Author

tlxpro commented Apr 20, 2025

Just tested again. Is nio.uv.fs_write equivalent to vim.uv.fs_write because the former doesn't work.

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