From e92be1c14b6b7346730e9a623fc35adb5b62f43b Mon Sep 17 00:00:00 2001
From: David Vacca <dvacca@fb.com>
Date: Wed, 21 Apr 2021 10:22:31 -0700
Subject: [PATCH] Add extra asserts and early deallocation in
 ReadableMapBuffer::importByteBufferAllocateDirect

Summary:
Add extra asserts and early deallocation in ReadableMapBuffer::importByteBufferAllocateDirect

changelog: [internal] internal

Reviewed By: sammy-SC

Differential Revision: D27904645

fbshipit-source-id: 075a007c4ec5e005b839add054bd68c233b65801
---
 .../common/mapbuffer/ReadableMapBuffer.cpp    | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/jni/react/common/mapbuffer/ReadableMapBuffer.cpp b/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/jni/react/common/mapbuffer/ReadableMapBuffer.cpp
index f8042542745..83d856abd0a 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/jni/react/common/mapbuffer/ReadableMapBuffer.cpp
+++ b/ReactAndroid/src/main/java/com/facebook/react/common/mapbuffer/jni/react/common/mapbuffer/ReadableMapBuffer.cpp
@@ -21,19 +21,29 @@ void ReadableMapBuffer::registerNatives() {
 
 jni::local_ref<jni::JByteBuffer>
 ReadableMapBuffer::importByteBufferAllocateDirect() {
-  // TODO: Using this method is safer than "importByteBuffer" because ByteBuffer
-  // memory will be deallocated once the "Java ByteBuffer" is deallocated. Next
-  // steps:
+  // TODO T83483191: Using this method is safer than "importByteBuffer" because
+  // ByteBuffer memory will be deallocated once the "Java ByteBuffer" is
+  // deallocated. Next steps:
   // - Validate perf of this method vs importByteBuffer
   // - Validate that there's no leaking of memory
+  react_native_assert(
+      (serializedData_ != nullptr && serializedDataSize_ != 0) &&
+      "Error serializedData_ is not initialized");
   auto ret = jni::JByteBuffer::allocateDirect(serializedDataSize_);
+  // TODO T83483191: avoid allocating serializedData_ when using
+  // JByteBuffer::allocateDirect
   std::memcpy(
       ret->getDirectBytes(), (void *)serializedData_, serializedDataSize_);
+
+  // Deallocate serializedData_ since it's not necessary anymore
+  delete[] serializedData_;
+  serializedData_ = nullptr;
+  serializedDataSize_ = 0;
   return ret;
 }
 
 jni::JByteBuffer::javaobject ReadableMapBuffer::importByteBuffer() {
-  // TODO: Reevaluate what's the best approach here (allocateDirect vs
+  // TODO T83483191: Reevaluate what's the best approach here (allocateDirect vs
   // DirectByteBuffer).
   //
   // On this method we should:
@@ -60,7 +70,6 @@ ReadableMapBuffer::~ReadableMapBuffer() {
     delete[] serializedData_;
     serializedData_ = nullptr;
   }
-  serializedDataSize_ = 0;
 }
 
 } // namespace react