Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mrousavy opened this issue Dec 7, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@mrousavy
Copy link
Contributor

mrousavy commented Dec 7, 2024

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:

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.

@mrousavy mrousavy added the enhancement New feature or request label Dec 7, 2024
@tmikov
Copy link
Contributor

tmikov commented Dec 10, 2024

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
Copy link
Contributor Author

Thank you! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants