From c2a59f93b3b89bdbaf53e1cb6571b5dad7d4dc18 Mon Sep 17 00:00:00 2001 From: Christoph Purrer Date: Fri, 17 Jul 2026 11:40:53 -0700 Subject: [PATCH] Use NSData for the JS->ObjC ArrayBuffer argument path Summary: The original implementation used `NSMutableData` for both directions of the JSI<->ObjC ArrayBuffer conversion. That is the wrong contract on the argument (JS -> ObjC/Native) side: an inbound buffer is owned by the caller, not the native module, and `NSMutableData` must own a resizable backing store it can `realloc`/`free`, so it cannot durably alias a foreign buffer. This diff keeps `NSMutableData` for the ObjC/Native -> JS (return) path, but switches the JS -> ObjC/Native (argument) path to the immutable `NSData`: - Codegen: `getParamObjCType` now maps `ArrayBufferTypeAnnotation` params to `NSData *` (return type stays `NSMutableData *`). - Runtime: `convertJSIArrayBufferToNSMutableData` is renamed to `convertJSIArrayBufferToNSData` and returns an immutable `NSData`. The bytes are still eagerly copied, which keeps the result safe to retain, store, or dispatch to another thread regardless of whether the source bytes were owned by JS or by a native `MutableBuffer`. - Updated the ObjC unit tests and the codegen `GenerateModuleHObjCpp` snapshot. Changelog: [INTERNAL] Differential Revision: D112582384 --- .../GenerateModuleObjCpp/serializeMethod.js | 2 +- .../GenerateModuleHObjCpp-test.js.snap | 4 ++-- .../core/iostests/RCTTurboModuleTests.mm | 23 +++++++++---------- .../ios/ReactCommon/RCTTurboModule.mm | 19 ++++++++------- .../RCTTurboModuleArrayBufferTests.mm | 15 ++++++------ 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js index 9f789ade20a0..928ddf0e4502 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleObjCpp/serializeMethod.js @@ -221,7 +221,7 @@ function getParamObjCType( return notStruct(wrapOptional('NSArray *', !nullable)); } case 'ArrayBufferTypeAnnotation': { - return notStruct(wrapOptional('NSMutableData *', !nullable)); + return notStruct(wrapOptional('NSData *', !nullable)); } } diff --git a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleHObjCpp-test.js.snap b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleHObjCpp-test.js.snap index 7612ea01b0f8..9fe9803acbd1 100644 --- a/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleHObjCpp-test.js.snap +++ b/packages/react-native-codegen/src/generators/modules/__tests__/__snapshots__/GenerateModuleHObjCpp-test.js.snap @@ -103,8 +103,8 @@ Map { @protocol NativeSampleTurboModuleSpec - (NSMutableData *)getArrayBuffer; -- (void)voidArrayBuffer:(NSMutableData *)arg; -- (void)voidNullableArrayBuffer:(NSMutableData * _Nullable)arg; +- (void)voidArrayBuffer:(NSData *)arg; +- (void)voidNullableArrayBuffer:(NSData * _Nullable)arg; - (void)promiseArrayBuffer:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject; diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm index b94f70a89a86..41612d2279b3 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/iostests/RCTTurboModuleTests.mm @@ -123,12 +123,11 @@ - (void)testInvokeTurboModuleWithNull OCMVerify(OCMTimes(1), [instance_ testMethodWhichTakesObject:nil]); } -// A JS ArrayBuffer converts to an NSMutableData that owns an independent copy of -// the bytes — NSMutableData cannot alias a foreign buffer, so the result stays -// valid and is safe to mutate after the source buffer is gone. This covers the -// ArrayBuffer-backed-by-native-MutableBuffer case, which is the one that could in -// principle have been aliased zero-copy. -- (void)testArrayBufferConvertsToIndependentNSMutableData +// A JS ArrayBuffer converts (on the argument path) to an immutable NSData that +// owns an independent copy of the bytes, so the result stays valid after the +// source buffer is gone. This covers the ArrayBuffer-backed-by-native-MutableBuffer +// case, which is the one that could in principle have been aliased zero-copy. +- (void)testArrayBufferConvertsToIndependentNSData { constexpr size_t kBufferSize = 64 * 1024; @@ -142,15 +141,15 @@ - (void)testArrayBufferConvertsToIndependentNSMutableData id converted = TurboModuleConvertUtils::convertJSIValueToObjCObject(*rt, facebook::jsi::Value(*rt, arrayBuffer), nullptr); - XCTAssertTrue([converted isKindOfClass:[NSMutableData class]]); - NSMutableData *data = (NSMutableData *)converted; + XCTAssertTrue([converted isKindOfClass:[NSData class]]); + NSData *data = (NSData *)converted; XCTAssertEqual(data.length, (NSUInteger)kBufferSize); XCTAssertEqual(*static_cast(data.bytes), 0xAB); - // Independent copy: mutating the NSMutableData must not write through to the - // source MutableBuffer. - *static_cast(data.mutableBytes) = 0xCD; - XCTAssertEqual(*buffer->data(), 0xAB, @"NSMutableData must not alias the source buffer"); + // Independent copy: mutating the source MutableBuffer must not write through to + // the NSData. + *buffer->data() = 0xCD; + XCTAssertEqual(*static_cast(data.bytes), 0xAB, @"NSData must not alias the source buffer"); } @end diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm index 504060fc1b23..ac0753934520 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm @@ -199,16 +199,15 @@ size_t size() const override }; } -// Copy the ArrayBuffer's bytes into an NSMutableData. A zero-copy wrap is not -// possible here: NSMutableData needs its own resizable backing store and cannot -// alias a foreign buffer (even via initWithBytesNoCopy:length:deallocator:, which -// copies eagerly for the mutable subclass). Copying also makes the result safe to -// retain in a block, store, or dispatch to another thread, regardless of whether -// the bytes were owned by JS (valid only for this callstack) or by a native -// MutableBuffer (which the JS ArrayBuffer may GC concurrently). -static NSMutableData *convertJSIArrayBufferToNSMutableData(jsi::Runtime &rt, const jsi::ArrayBuffer &value) +// Copy the ArrayBuffer's bytes into an immutable NSData. An inbound buffer is +// owned by the caller, not the native module, so NSData (not NSMutableData) is +// the correct read-only contract. Copying makes the NSData self-contained and +// safe to retain in a block, store, or dispatch to another thread, regardless of +// whether the bytes were owned by JS (valid only for this callstack) or by a +// native MutableBuffer (which the JS ArrayBuffer may GC concurrently). +static NSData *convertJSIArrayBufferToNSData(jsi::Runtime &rt, const jsi::ArrayBuffer &value) { - return [NSMutableData dataWithBytes:value.data(rt) length:value.size(rt)]; + return [NSData dataWithBytes:value.data(rt) length:value.size(rt)]; } id convertJSIValueToObjCObject( @@ -241,7 +240,7 @@ id convertJSIValueToObjCObject( return convertJSIFunctionToCallback(runtime, o.getFunction(runtime), jsInvoker); } if (o.isArrayBuffer(runtime)) { - return convertJSIArrayBufferToNSMutableData(runtime, o.getArrayBuffer(runtime)); + return convertJSIArrayBufferToNSData(runtime, o.getArrayBuffer(runtime)); } return convertJSIObjectToNSDictionary(runtime, o, jsInvoker, useNSNull); } diff --git a/packages/rn-tester/RNTesterUnitTests/RCTTurboModuleArrayBufferTests.mm b/packages/rn-tester/RNTesterUnitTests/RCTTurboModuleArrayBufferTests.mm index fd242c2de83d..156af187618d 100644 --- a/packages/rn-tester/RNTesterUnitTests/RCTTurboModuleArrayBufferTests.mm +++ b/packages/rn-tester/RNTesterUnitTests/RCTTurboModuleArrayBufferTests.mm @@ -105,16 +105,17 @@ @implementation RCTTestArrayBufferTurboModule RCT_EXPORT_MODULE() -RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSMutableData *, testMethodWhichMutatesArrayBuffer : (NSMutableData *)buffer) +RCT_EXPORT_SYNCHRONOUS_TYPED_METHOD(NSMutableData *, testMethodWhichTransformsArrayBuffer : (NSData *)buffer) { - auto *bytes = static_cast(buffer.mutableBytes); - for (NSUInteger i = 0; i < buffer.length; ++i) { + NSMutableData *result = [buffer mutableCopy]; + auto *bytes = static_cast(result.mutableBytes); + for (NSUInteger i = 0; i < result.length; ++i) { bytes[i] = static_cast((i + 1) * 10); } - return buffer; + return result; } -RCT_EXPORT_METHOD(testMethodWhichStoresArrayBuffer : (NSMutableData *)payload) +RCT_EXPORT_METHOD(testMethodWhichStoresArrayBuffer : (NSData *)payload) { self.lastReceivedPayload = [payload copy]; } @@ -169,8 +170,8 @@ - (void)testSyncArrayBufferRoundTrip auto result = module.invokeObjCMethod( *rt, ArrayBufferKind, - "testMethodWhichMutatesArrayBuffer", - @selector(testMethodWhichMutatesArrayBuffer:), + "testMethodWhichTransformsArrayBuffer", + @selector(testMethodWhichTransformsArrayBuffer:), args, 1);