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

Using buffers with non-blocking FFI calls is unsound #12341

Open
andreubotella opened this issue Oct 6, 2021 · 7 comments
Open

Using buffers with non-blocking FFI calls is unsound #12341

andreubotella opened this issue Oct 6, 2021 · 7 comments
Assignees
Labels
bug Something isn't working correctly FFI Related to Foreign Function Interface APIs

Comments

@andreubotella
Copy link
Contributor

andreubotella commented Oct 6, 2021

Recently non-blocking FFI was made possible with PR #12274, and the use of buffers with FFI was made possible with PR #12335. However, currently only non-shared ArrayBuffers can be used with FFI (due to denoland/serde_v8#3), which means they can't be shared across threads. Given that non-blocking FFI is implemented with a tokio blocking task, which runs in a separate thread, it follows that using buffers with non-blocking FFI is automatically unsound.

See denoland/serde_v8#47 for why this did not cause any compilation errors.

@andreubotella andreubotella changed the title Using buffers with non-blocking FFI calls is UB Using buffers with non-blocking FFI calls is unsound Oct 6, 2021
@bartlomieju
Copy link
Member

CC @piscisaureus

@rracariu
Copy link

rracariu commented Oct 7, 2021

While arguments here are valid, I think they are not applicable to FFI, where you could end-up with memory corruption and/or access violation just by calling a function with the wrong arguments.

FFI should be viewed as unsafe/here be dragons from the get go, and with that said no enforcing on the semantics can be assumed. If FFI is to be completed to the level where you can call any C functions, then it will need to support all pointer types from C including pointer-to-pointer, which means that the JS semantics would be of no use when dealing with those, as you could expect that a C function would change the content of the Buffer concurrently.

If the plugin/extension interface was kept, then yes - there could be enforcement on the semantics and what one is expected to do.

@andreubotella
Copy link
Contributor Author

While arguments here are valid, I think they are not applicable to FFI, where you could end-up with memory corruption and/or access violation just by calling a function with the wrong arguments.

FFI should be viewed as unsafe/here be dragons from the get go, and with that said no enforcing on the semantics can be assumed. If FFI is to be completed to the level where you can call any C functions, then it will need to support all pointer types from C including pointer-to-pointer, which means that the JS semantics would be of no use when dealing with those, as you could expect that a C function would change the content of the Buffer concurrently.

If the plugin/extension interface was kept, then yes - there could be enforcement on the semantics and what one is expected to do.

My point was that simply reading or writing to the buffer from the FFI library would be UB from Rust's perspective, because it would be two threads having mutable access to that buffer without any synchronization. The only way that it would not be UB is if the FFI library did nothing with the pointer or just used it as an identifier.

@rracariu
Copy link

rracariu commented Oct 7, 2021

My point was that simply reading or writing to the buffer from the FFI library would be UB from Rust's perspective, because it would be two threads having mutable access to that buffer without any synchronization. The only way that it would not be UB is if the FFI library did nothing with the pointer or just used it as an identifier.

Right, in that case for non-blocking calls the buffer would probably be transferred to the FFI trampoline, kind of what you can do with Workers and transferables.

@stale
Copy link

stale bot commented Dec 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 6, 2021
@stale stale bot closed this as completed Dec 13, 2021
@littledivy littledivy removed the stale label Feb 12, 2022
@littledivy littledivy reopened this Feb 12, 2022
@littledivy
Copy link
Member

Related proposal #13550

@stale
Copy link

stale bot commented Apr 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 14, 2022
@stale stale bot closed this as completed Apr 21, 2022
@littledivy littledivy added bug Something isn't working correctly FFI Related to Foreign Function Interface APIs and removed stale labels Apr 21, 2022
@littledivy littledivy reopened this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly FFI Related to Foreign Function Interface APIs
Projects
None yet
Development

No branches or pull requests

4 participants