facebook / facebook/hermes

Support for ArrayBuffer::isMutableBuffer() and ArrayBuffer::getMutableBuffer()

Open
#1,578 9 comments 6 reactions 0 assignees View on GitHub
enhancement
Dominant language
JavaScript
Stars
11.3k
Forks
859
Avg merge
1h 30m
Merged PRs (30d)
3

Description

## Problem

In [Nitro](https://nitro.margelo.com), array buffers are supported types for native modules.

Imagine the following native nitro module:

```ts
const seedArrayBuffer = myNitroModule.createRandomSeed()
const keyString = myNitroModule.getEncryptionKey(seedArrayBuffer)
```

On the native side of `getEncryptionKey`, we can **safely** access the `ArrayBuffer`'s data without any problems just fine because it is synchronous and we are running on the JS thread:

```cpp
auto arrayBuffer = args[0].getObject(runtime).getArrayBuffer(runtime);
void* data = arrayBuffer.data(runtime);
```

..but as soon as we make this function **asynchronous**:

```ts
const seedArrayBuffer = myNitroModule.createRandomSeed()
const keyString = await myNitroModule.getEncryptionKey(seedArrayBuffer)
```

We can no longer **safely** access the `ArrayBuffer`'s data because we are running on a separate Thread.

> Nitro will convert the `jsi::ArrayBuffer` to a custom Nitro type that basically tells you to not access it on a different Thread and will throw if you try to do so. So **the user is always forced to make a copy of the data before switching to a different Thread.**

I think it'd be a cool API to get access to the underlying `std::shared_ptr` **if it has one** (not every `ArrayBuffer` is created with one):

```cpp
auto arrayBuffer = args[0].getObject(runtime).getArrayBuffer(runtime);
auto mutableBuffer = arrayBuffer.getMutableBuffer(runtime);
std::thread([=]() {
void* data = mutableBuffer.data();
});
```

> Note: This could cause data race issues if not used correctly. I think it's up to the user to guard against that (either via Mutexes, or just praying at night that their APIs will not be misused)

## Solution

- `jsi::ArrayBuffer::isMutableBuffer(..)`
- `jsi::ArrayBuffer::getMutableBuffer(..)`
- `jsi::ArrayBuffer::asMutableBuffer(..)` (?)

## Additional Context

In Nitro, I currently solved it like this:

- [The `nitro::ArrayBuffer` base class](https://github.com/mrousavy/nitro/blob/3ccafbc314c9b9af19825fb2d0fa601de1d52edb/packages/react-native-nitro-modules/cpp/core/ArrayBuffer.hpp#L20-L56)
- [The `nitro::NativeArrayBuffer` class](https://github.com/mrousavy/nitro/blob/3ccafbc314c9b9af19825fb2d0fa601de1d52edb/packages/react-native-nitro-modules/cpp/core/ArrayBuffer.hpp#L58-L91), which allows you to access data from any Thread and is **owning**
- [The `nitro::JSArrayBuffer` class](https://github.com/mrousavy/nitro/blob/3ccafbc314c9b9af19825fb2d0fa601de1d52edb/packages/react-native-nitro-modules/cpp/core/ArrayBuffer.hpp#L93-L127), which only allows you to access data on the JS Thread

If I receive an `ArrayBuffer` from JS, it is currently **always** a `nitro::JSArrayBuffer`. I would love to look into it and unwrap the `jsi::MutableBuffer` from it so I can optionally also pass the user a `nitro::NativeArrayBuffer` instead though.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.