Skip to content

Commit fff4994

Browse files
generatedunixname1608173377072046meta-codesync[bot]
authored andcommitted
Harden on-demand JS segment load path against missing cache file (#57855)
Summary: Pull Request resolved: #57855 Harden the on-demand (lazily code-split) JS segment load path against a missing cache file. When RN lazily loads an on-demand JS segment, the file lives in an OS-purgeable / LRU-evictable on-demand cache and can already be gone by the time the segment is registered. On a missing file `JSBigFileString::fromPath` throws `std::runtime_error("... Could not open file: ...")`, and `ReactInstance::registerSegment` had no failure handling, so the throw surfaced as an unhandled JS exception and crashed the app. Fix (V4, addresses pieterdb's latest review feedback): - `JSBigFileString::fromPath`'s throwing contract is now documented in the header: it throws `std::runtime_error` when the file cannot be opened or read, and callers loading files that may legitimately be gone (an OS-purgeable / LRU-evictable cache) must catch and degrade gracefully. No behavior change to `fromPath` and no new factory. - `ReactInstance::registerSegment` builds the segment buffer on the calling thread (outside the `scheduleWork` callback) inside a try/catch. On the thrown exception it logs an ERROR and returns early, so a missing segment never schedules evaluation and the throw no longer escapes. The buffer is carried into the callback as a `std::shared_ptr<const JSBigFileString>` (`RuntimeScheduler` callbacks are copyable `std::function`s; `evaluateJavaScript` takes `const shared_ptr<const Buffer>&`). This keeps file I/O off the JS thread. - Dropped the inline `access(R_OK)` pre-check: the `open()` failure inside `fromPath` is the single source of truth, removing the redundant check and its TOCTOU window. Changelog: [iOS][Fixed] - Avoid an unhandled exception in bridgeless `ReactInstance::registerSegment` when an on-demand JS segment file is missing from the cache at lazy-load time Reviewed By: zeyap, javache Differential Revision: D114761643 fbshipit-source-id: 8c8e0bd8ae5886953a77d1596181d83d8967df67
1 parent 44e590f commit fff4994

2 files changed

Lines changed: 38 additions & 20 deletions

File tree

packages/react-native/ReactCommon/cxxreact/JSBigString.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,11 @@ class RN_EXPORT JSBigFileString : public JSBigString {
140140
size_t size() const override;
141141
int fd() const;
142142

143+
// Throws std::runtime_error when the file at sourceURL cannot be opened or
144+
// read (e.g. it is missing). Callers that load files which may legitimately
145+
// be gone at load time (such as an OS-purgeable / LRU-evictable cache) must
146+
// catch this and degrade gracefully rather than let it surface as an
147+
// unhandled exception.
143148
static std::unique_ptr<const JSBigFileString> fromPath(const std::string &sourceURL);
144149

145150
private:

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -378,26 +378,39 @@ void ReactInstance::registerSegment(
378378
const std::string& segmentPath) {
379379
LOG(WARNING) << "Starting to run ReactInstance::registerSegment with segment "
380380
<< segmentId;
381-
runtimeScheduler_->scheduleWork([=](jsi::Runtime& runtime) {
382-
TraceSection s("ReactInstance::registerSegment");
383-
auto tag = std::to_string(segmentId);
384-
auto script = JSBigFileString::fromPath(segmentPath);
385-
if (script->size() == 0) {
386-
throw std::invalid_argument(
387-
"Empty segment registered with ID " + tag + " from " + segmentPath);
388-
}
389-
390-
ReactMarker::logTaggedMarker(
391-
ReactMarker::REGISTER_JS_SEGMENT_START, tag.c_str());
392-
LOG(WARNING) << "Starting to evaluate segment " << segmentId
393-
<< " in ReactInstance::registerSegment";
394-
runtime.evaluateJavaScript(
395-
std::move(script), getSyntheticBundlePath(segmentId));
396-
LOG(WARNING) << "Finished evaluating segment " << segmentId
397-
<< " in ReactInstance::registerSegment";
398-
ReactMarker::logTaggedMarker(
399-
ReactMarker::REGISTER_JS_SEGMENT_STOP, tag.c_str());
400-
});
381+
// Build the segment buffer off the JS thread: there's no need to block the
382+
// JS thread on file I/O. The segment lives in an OS-purgeable /
383+
// LRU-evictable on-demand cache and can already be gone by the time we get
384+
// here, in which case fromPath throws. Catch it and return early so a
385+
// missing segment degrades gracefully instead of surfacing as an unhandled
386+
// exception.
387+
std::shared_ptr<const JSBigFileString> script;
388+
try {
389+
script = JSBigFileString::fromPath(segmentPath);
390+
} catch (const std::exception& e) {
391+
LOG(ERROR) << "ReactInstance::registerSegment - could not load segment "
392+
<< segmentId << " from " << segmentPath << ": " << e.what();
393+
return;
394+
}
395+
if (script->size() == 0) {
396+
throw std::invalid_argument(
397+
"Empty segment registered with ID " + std::to_string(segmentId) +
398+
" from " + segmentPath);
399+
}
400+
runtimeScheduler_->scheduleWork(
401+
[script = std::move(script), segmentId](jsi::Runtime& runtime) {
402+
TraceSection s("ReactInstance::registerSegment");
403+
auto tag = std::to_string(segmentId);
404+
ReactMarker::logTaggedMarker(
405+
ReactMarker::REGISTER_JS_SEGMENT_START, tag.c_str());
406+
LOG(WARNING) << "Starting to evaluate segment " << segmentId
407+
<< " in ReactInstance::registerSegment";
408+
runtime.evaluateJavaScript(script, getSyntheticBundlePath(segmentId));
409+
LOG(WARNING) << "Finished evaluating segment " << segmentId
410+
<< " in ReactInstance::registerSegment";
411+
ReactMarker::logTaggedMarker(
412+
ReactMarker::REGISTER_JS_SEGMENT_STOP, tag.c_str());
413+
});
401414
}
402415

403416
namespace {

0 commit comments

Comments
 (0)