feat: ffi string and buffer support

[eliassjogreen]

Follow up on #11152 adding support for strings and buffer arguments and returns (and a tests for other argument types)

[ReddikHaien]

I've probably overlooked it, but wouldn't it be necessary with a bound check on the buffer to prevent someone from passing a invalid length for the raw pointer?

[bartlomieju]

I believe this code threw TypeError previously in dlopenInvalidArguments but now it only throws Error

[konsumer]

Anything I can do to help get this in? I am not super-strong with rust, but I have several projects that need mem-buffers, in deno FFI, so I am super-motivated. I can verify this PR works on Windows, Linux, and OSX (all on x86_64.)

[cztomsik]

FYI I have added few comments here https://github.com/denoland/deno/pull/11648/files#r713404236
I believe it would be nice to support *mut c_char result type even with into_raw() leaking memory because you can always wrap the return in finally { lib.free_string(s) }

[rracariu]

Looking fwd. to get this PR in. But it is stale for a long time.

Can it be simplified to only deal with buffers for now? This (with #12274 merged) would unlock the bare minimum for replacing the plugin API.

[kriszyp]

I'll just add another vote that I think the most essential elements for really opening the doors for native/foreign plugins are passing buffers and async support (some way to enqueue a callback on the event queue). Strings are not necessary; maybe a convenience, but JS already has facilities for string encoding/decoding that work fine. To me, adding support for buffers & async to FFI is the most critical/valuable outstanding feature/issue in Deno.

[cztomsik]

Oh, I just asked this on discord - I'd be okay with buffers as well (if they can be passed as mut) because then it's possible to have at least hacky String <-> String communication in a form of strlen(string_id) -> usize and copy(string_id, dest_buf)

[rotu]

I think allowing strings also opens up another can of worms. As shown in #12226, a Javascript DOMString can legally contain lone surrogates. This is okay in C++ and Python, but not in Rust and Swift. Buffers don't have this issue.

[bartlomieju]

I forked off this PR to #12335, we're gonna support only buffer-as-arg in the first pass

[bartlomieju]

@rracariu @kriszyp @cztomsik @rotu buffer-as-arg is now available on canary, see #12335 for details. I'll work on documentation as per denoland/manual#106

[rracariu]

@rracariu @kriszyp @cztomsik @rotu buffer-as-arg is now available on canary, see #12335 for details. I'll work on documentation as per denoland/manual#106

Without return buffers one would not be able to return complex structures, which severely limits the things you could do.

In C there is no buffer type - so strictly speaking this concept is mapped to u8* by Deno, but that is a convention.
So this means that the buffer type could expand on this convention until proper pointer types (in/out) are supported by the FFI interface.

What I mean by that is that the FFI return type could be a buffer_return FFI type which is a struct {size_t size; size_t ptr} that is mapped as a FfiBuffer type in Deno/Typescript. This FfiBuffer would have the byte contents for the pointer, but expose the pointer value as well, like interface FfiBuffer {readonly data: ArrayBuffer; readonly ptr: BigInt | number }. Using the ptr value one could call a free function to free the memory over in the FFI land.

[kriszyp]

Without return buffers one would not be able to return complex structures, which severely limits the things you could do.

I was assuming, at least, that passing in a buffer as an argument was by reference (I hope this doesn't copy the buffer before handing it to C function), so the called function would be able to freely modify the contents of the buffer to construct/represent/communicate complex structures back to JS. If my understanding is correct (and certainly correct me if I am wrong), the limitation would be that allocation can't be done on the C, but needs to be done in JS (creating an TypedArray/ArrayBuffer), that can then be used for JS/C interaction. I would consider that to be a very reasonable limitation.

[bartlomieju]

Without return buffers one would not be able to return complex structures, which severely limits the things you could do.

I was assuming, at least, that passing in a buffer as an argument was by reference (I hope this doesn't copy the buffer before handing it to C function), so the called function would be able to freely modify the contents of the buffer to construct/represent/communicate complex structures back to JS. If my understanding is correct (and certainly correct me if I am wrong), the limitation would be that allocation can't be done on the C, but needs to be done in JS (creating an TypedArray/ArrayBuffer), that can then be used for JS/C interaction. I would consider that to be a very reasonable limitation.

That is correct. @rracariu has valid concerns and we'll try to address them for v1.16, having to explicitly call foreign lib to free a buffer is certainly better than leaking it.

[kriszyp]

BTW, the current ffi API (even including the buffer return limitation), almost exactly matches the V8 fast-api-calls interface. Was that intentional; and does deno/rusty use or intend to use V8's fast-api-calls interface (https://github.com/v8/v8/blob/master/include/v8-fast-api-calls.h)?

[bartlomieju]

BTW, the current ffi API (even including the buffer return limitation), almost exactly matches the V8 fast-api-calls interface. Was that intentional; and does deno/rusty use or intend to use V8's fast-api-calls interface (https://github.com/v8/v8/blob/master/include/v8-fast-api-calls.h)?

Yes, we plan to use V8's Fast API but for our internal "op" mechanism

[rracariu]

I was assuming, at least, that passing in a buffer as an argument was by reference

I assumed the contrary given that the code is actually using const* u8 as the argument (https://github.com/denoland/deno/pull/12335/files#diff-5cab5c857e7d36ba5f5b617a7c541b75980f1ed6460ad00706b45b446346f339R221).

But a quick test proved that you can actually change the buffer from the C function. @bartlomieju Is this intentional?

[bartlomieju]

I was assuming, at least, that passing in a buffer as an argument was by reference

I assumed the contrary given that the code is actually using const* u8 as the argument (https://github.com/denoland/deno/pull/12335/files#diff-5cab5c857e7d36ba5f5b617a7c541b75980f1ed6460ad00706b45b446346f339R221).

But a quick test proved that you can actually change the buffer from the C function. @bartlomieju Is this intentional?

That is the type that is handled by Deno's FFI API, however when the boundary is crossed it actually passes a pointer. I will clean up the code before 1.15 as there's one bug there anyway.

[piscisaureus]

I was assuming, at least, that passing in a buffer as an argument was by reference

Yes, passing buffers should be done by reference.

[bartlomieju]

I'm going to close this PR now as it has become quite stale. We will try to get buffer-as-return-value support for v1.16

[wobsoriano]

Is it possible to return strings as value now?

[cztomsik]

You can return CString::new("foo").unwrap().into_raw() and then in JS new UnsafePointerView(ptr).getCString() to decode that string but you also have to free that memory eventually or it will leak.

Either you need extra ffi function which will free the pointer when you're done or you can reuse the same (mutex or thread-local) CString and kind of "reuse" it every time and never really free it

[wobsoriano]

You can return CString::new("foo").unwrap().into_raw() and then in JS new UnsafePointerView(ptr).getCString() to decode that string but you also have to free that memory eventually or it will leak.

Either you need extra ffi function which will free the pointer when you're done or you can reuse the same (mutex or thread-local) CString and kind of "reuse" it every time and never really free it

That worked! Thanks. Didn't know UnsafePointerView returns methods

[cztomsik]

I found out recently as well, inspecting the test cases actually :)
https://github.com/denoland/deno/tree/main/test_ffi