Suggestion: Add Deno.UnsafeOwnerPointer for owned memory handling

[aapoalas]

EDIT: Somewhat better organized, though wordier proposal text available here.

At the very least with C++ FFI, it is common for some FFI functions to expect a pointer to a slice of memory that will be used as the "return value" of the function.

eg. Given the following C++ struct and class method:

struct Address {
  uin32_t first_id;
  uint64_t second_id;
}

class SomeClass {
  public:
  Address address() const;
}

The FFI calling convention (at least with System V ABI, ie. on Linux) is actually like this (using Rust to elaborate):

extern "C" {
  fn SomeClass_address(*mut Address, *const SomeClass);
}

ie. The return value is void and the the C++ return value for Address has instead become a prepended mutable pointer. This pointer is expected to point to a memory large enough to write Address into, in this case 40 bytes.

In Deno, this can be done as follows:

const addressData = new Uint8Array(40);
const addressPointer = Deno.UnsafePointer.of(addressData);

but now one must be careful to never let addressData be garbage collected while addressPointer still lives and is used.

An obvious way to do this is this:

class UnsafeOwnedPointer extends Deno.UnsafePointer {
  data: Uint8Array;
  /**
   * @param size Size of owner memory in bytes
   */
  constructor(size: number) {
    const data = new Uint8Array(size);
    super(Deno.UnsafePointer.of(data).value); // TODO: Avoid allocation of UnsafePointer by using the of op directly
    this.data = data;
  }
}

Now as long as the pointer lives, the data lives.

Memory allocation and deallocation

With an OwnedPointer, or the associated Uint8Array, it's clear that Deno has been the one to allocate the memory and should be the one to deallocate it. However, it is quite possible that whatever is saved behind the OwnedPointer has a destructor. Calling that destructor is obviously not automatically done by OwnedPointer, nor is that the point.

Calling a possible destructor needs to be done by the programmer. But at the very least with C++ this does not pose any special issue (beyond requiring manual steps), C++ does not try to free the memory of the data Uint8Array or any such nonsense. The destructor will (or should) only take any necessary steps to clean up memory that is not itself included in the data itself. So f.ex. pointers within the data pointing to internally allocated data, or calls to notify some library internal trackers about a removal of a class instance, these will be done by the destructor. Actual deallocation of the owned memory is left up to whoever called the destructor, as it should be.

Internal vs user-land

It is easy to argue that UnsafeOwnedPointer can be implemented as a user-land extension and thus does not need to be included in Deno itself. I would argue that this is still a very useful thing to include in Deno core. For one, the unnecessary allocation of UnsafePointer can be avoided by directly using the of op. Additionally, this may be a fairly regular use case, in which case first-class support is not at all a bad idea.

[aapoalas]

In a discussion related to (un)safety of passing buffers into FFI calls marked nonblocking @andreubotella suggested an AsyncBuffer object that would own an ArrayBuffer and work as a parameter in FFI calls. The reasoning for this was that a dedicated object would facilitate detaching the buffer when it is being sent to a Tokio spawn thread for the nonblocking FFI call. The idea for the AsyncBuffer was inspired by BYOB streams.

Currently, sending a buffer into a nonblocking FFI call is kind of UB. JavaScript retains the ability to read and write into the buffer while in Rust terms the buffer is sent exclusively to another thread (I think?). This is fertile ground for data races and UB of all sorts.

This ties nicely into this UnsafeOwnedPointer proposal: The UnsafeOwnedPointer could be made to work like AsyncBuffer in nonblocking FFI calls in that it would detach the buffer when the FFI call is initiated and would replace its internal buffer when the async call finishes with a new buffer pointing to the same backing memory. This would eliminate the possibility of data races and would facilitate correct Rust language semantics for sending the buffer to the Tokio spawn thread.

The current UnsafePointer API shows itself as a potential issue here, specifically the UnsafePointer.of static method. If it is possible for JavaScript to get a pointer value to its own ArrayBuffer, then it can read data from that pointer using UnsafePointerView even when the buffer is detached. Based on this I might argue that eventually the FFI pointers should be split into two cases:

1. OwnedPointer: Creation would usually be through the constructor with only length provided. Optionally provide a static method from to copy from (or unsafely take ownership of) a given buffer, facilitating pre-initialized memory use-cases.
2. ForeignPointer: This is the current UnsafePointer, except that no creation would be allowed. These would only appear as return values from FFI calls. Reading is always done using a PointerView class.

As it comes to passing ArrayBuffers into FFI calls directly: For synchronous functions this could still be allowed, but for nonblocking calls I believe it should throw an error. There is no way to detach the buffer and return it to the caller once the Promise resolves, and there is no way to guarantee memory safety while the Promise is ongoing.

[aapoalas]

I added a more wordy proposal with some code examples here: https://github.com/aapoalas/deno-stable-ffi

[littledivy]

Linking #12341 (comment) here. I think that FFI should be as "raw" and directly interoperable with JavaScript values as possible.