From fa082796a586695deaf0916a34de4f84f9330cc0 Mon Sep 17 00:00:00 2001
From: Ashok Menon <ashokmenon@fb.com>
Date: Fri, 16 Dec 2016 06:03:22 -0800
Subject: [PATCH] Make file backed strings dup the provided file descriptor.

Reviewed By: mhorowitz

Differential Revision: D4326645

fbshipit-source-id: 2741f1fead4f42ae76787f8a70c1e787445b827d
---
 ReactCommon/cxxreact/Executor.cpp          |  3 ++-
 ReactCommon/cxxreact/Executor.h            | 14 ++++++++++++--
 ReactCommon/cxxreact/tests/jsbigstring.cpp |  2 --
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/ReactCommon/cxxreact/Executor.cpp b/ReactCommon/cxxreact/Executor.cpp
index 1fec2ca21f7..adbf2058749 100644
--- a/ReactCommon/cxxreact/Executor.cpp
+++ b/ReactCommon/cxxreact/Executor.cpp
@@ -9,6 +9,7 @@
 #include <sys/stat.h>
 
 #include <folly/Memory.h>
+#include <folly/ScopeGuard.h>
 
 namespace facebook {
 namespace react {
@@ -51,7 +52,7 @@ std::unique_ptr<const JSBigOptimizedBundleString> JSBigOptimizedBundleString::fr
   uint8_t encoding;
   struct stat fileInfo;
   int fd = -1;
-  SCOPE_FAIL { CHECK(fd == -1 || ::close(fd) == 0); };
+  SCOPE_EXIT { CHECK(fd == -1 || ::close(fd) == 0); };
 
   {
     auto metaPath = bundlePath + UNPACKED_META_PATH_SUFFIX;
diff --git a/ReactCommon/cxxreact/Executor.h b/ReactCommon/cxxreact/Executor.h
index cb7e0015741..5eaa853d998 100644
--- a/ReactCommon/cxxreact/Executor.h
+++ b/ReactCommon/cxxreact/Executor.h
@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <fcntl.h>
 #include <functional>
 #include <memory>
 #include <string>
@@ -9,6 +10,7 @@
 
 #include <sys/mman.h>
 
+#include <folly/Exception.h>
 #include <folly/dynamic.h>
 
 #include "JSModulesUnbundle.h"
@@ -152,9 +154,13 @@ class JSBigFileString : public JSBigString {
 public:
 
   JSBigFileString(int fd, size_t size, off_t offset = 0)
-    : m_fd   {fd}
+    : m_fd   {-1}
     , m_data {nullptr}
   {
+    folly::checkUnixError(
+      m_fd = dup(fd),
+      "Could not duplicate file descriptor");
+
     // Offsets given to mmap must be page aligend. We abstract away that
     // restriction by sending a page aligned offset to mmap, and keeping track
     // of the offset within the page that we must alter the mmap pointer by to
@@ -216,11 +222,15 @@ public:
   };
 
   JSBigOptimizedBundleString(int fd, size_t size, const uint8_t sha1[20], Encoding encoding) :
-    m_fd(fd),
+    m_fd(-1),
     m_size(size),
     m_encoding(encoding),
     m_str(nullptr)
   {
+    folly::checkUnixError(
+      m_fd = dup(fd),
+      "Could not duplicate file descriptor");
+
     memcpy(m_hash, sha1, 20);
   }
 
diff --git a/ReactCommon/cxxreact/tests/jsbigstring.cpp b/ReactCommon/cxxreact/tests/jsbigstring.cpp
index 31e9adc5fa4..75f9a9cda22 100644
--- a/ReactCommon/cxxreact/tests/jsbigstring.cpp
+++ b/ReactCommon/cxxreact/tests/jsbigstring.cpp
@@ -37,7 +37,6 @@ TEST(JSBigFileString, MapWholeFileTest) {
   JSBigFileString bigStr {fd, size};
 
   // Test
-  ASSERT_EQ(fd, bigStr.fd());
   ASSERT_STREQ(data.c_str(), bigStr.c_str());
 }
 
@@ -53,7 +52,6 @@ TEST(JSBigFileString, MapPartTest) {
   JSBigFileString bigStr {fd, needle.size(), offset};
 
   // Test
-  ASSERT_EQ(fd, bigStr.fd());
   ASSERT_EQ(needle.length(), bigStr.size());
   for (unsigned int i = 0; i < needle.length(); ++i) {
     ASSERT_EQ(needle[i], bigStr.c_str()[i]);