search

denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
98.27k stars 5.41k forks source link

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

Open andreubotella opened 3 years ago

andreubotella commented 3 years ago

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.

bartlomieju commented 3 years ago

CC @piscisaureus

rracariu commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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[bot] commented 2 years ago

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.

littledivy commented 2 years ago

Related proposal https://github.com/denoland/deno/issues/13550

stale[bot] commented 2 years ago

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.