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

[mrousavy]

Problem

In Nitro, array buffers are supported types for native modules.

Imagine the following native nitro module:

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:

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

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

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<jsi::MutableBuffer> if it has one (not every ArrayBuffer is created with one):

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
• The nitro::NativeArrayBuffer class, which allows you to access data from any Thread and is owning
• The nitro::JSArrayBuffer class, 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.

[tmikov]

The team discussed this and we think it is a good idea. Thanks for proposing it! It is now in the short term TODO list.

[mrousavy]

Thank you! 🙏

[mrousavy]

Question; does every ArrayBuffer have a jsi::MutableBuffer, even those created from JS?

Or does this only apply to ArrayBuffers that were created with the jsi::MutableBuffer API?

[tmikov]

Question; does every ArrayBuffer have a jsi::MutableBuffer, even those created from JS? Or does this only apply to ArrayBuffers that were created with the jsi::MutableBuffer API?

Technically, today no ArrayBuffers have a jsi::MutableBuffer, since there is no way to obtain it back. But I get what you are asking. When we add ArrayBuffer::getMutableBuffer, will it work on any ArrayBuffer?

I don't think we envisioned that it would, but at the same time I think that it could work.

Do you have a motivating use case?

[mrousavy]

Do you have a motivating use case?

Well, there's a nice use-case: https://github.com/mrousavy/nitro/issues/601

I cannot access .data() on the jsi::ArrayBuffer from a different Thread, so I'd HAVE to copy it. (yes synchronisation is the user's responsibility) If it was a jsi::MutableBuffer, it'd be safer I guess.

[erights]

Please see stage 2.7 proposal https://github.com/tc39/proposal-immutable-arraybuffer with shim at https://github.com/endojs/endo/tree/master/packages/immutable-arraybuffer . The proposal is also natively implemented by Moddable's XS engine, passing all the test262 tests in the test262 PR that will add these tests.

How much overlap is there between Immutable ArrayBuffers and what is proposed here? Attn @naugtur @kriskowal

[mrousavy]

Hey - is there any update on this feature request? I would love to implement this myself and create a PR, but unfortunately I'm just not smart enough to find my way around the Hermes codebase.

I'm wondering if I should just wrap it in a jsi::NativeState and slap that on the jsi::Object in the meantime..

[mrousavy]

I created a PoC PR that implements this: https://github.com/facebook/hermes/pull/1733

[erights]

Why not just implement the Immutable ArrayBuffers proposal?