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

Flush RN task queue with invokeAsync #4389

Merged
merged 6 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ x.x.x Release notes (yyyy-MM-dd)
* None.

### Fixed
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?)
* Fixed issue where React Native apps would sometimes show stale Realm data until the user interacted with the app UI ([#4389](https://github.com/realm/realm-js/issues/4389), since v10.0.0)
* None.

### Compatibility
Expand Down
4 changes: 2 additions & 2 deletions example/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ PODS:
- React-jsi (= 0.66.4)
- React-logger (= 0.66.4)
- React-perflogger (= 0.66.4)
- RealmJS (10.14.0):
- RealmJS (10.16.0):
- GCDWebServer
- React
- Yoga (1.14.0)
Expand Down Expand Up @@ -535,7 +535,7 @@ SPEC CHECKSUMS:
React-RCTVibration: e3ffca672dd3772536cb844274094b0e2c31b187
React-runtimeexecutor: dec32ee6f2e2a26e13e58152271535fadff5455a
ReactCommon: 57b69f6383eafcbd7da625bfa6003810332313c4
RealmJS: 5a939376f3bdd94708abe2326d33d8f935b9fb37
RealmJS: 8a3478957315c29cdc0b3f958f2e370d22330b2d
Yoga: e7dc4e71caba6472ff48ad7d234389b91dadc280
YogaKit: f782866e155069a2cca2517aafea43200b01fd5a

Expand Down
36 changes: 33 additions & 3 deletions react-native/ios/RealmReact/RealmReact.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#import <React/RCTBridge+Private.h>
#import <React/RCTJavaScriptExecutor.h>
#include <ReactCommon/CallInvoker.h>

#import <objc/runtime.h>
#import <arpa/inet.h>
Expand Down Expand Up @@ -51,6 +52,8 @@ - (JSContext *)context;
@interface RCTBridge (Realm_RCTCxxBridge)
- (JSGlobalContextRef)jsContextRef;
- (void *)runtime;
// Expose the CallInvoker so that we can call `invokeAsync`
- (std::shared_ptr<facebook::react::CallInvoker>)jsCallInvoker;
@end

extern "C" JSGlobalContextRef RealmReactGetJSGlobalContextForExecutor(id executor, bool create) {
Expand Down Expand Up @@ -83,6 +86,8 @@ @interface RealmReact () <RCTBridgeModule>

@implementation RealmReact {
NSMutableDictionary *_eventHandlers;
// Keep track of whether we are already waiting for the React Native UI queue to be flushed asynchronously
bool waitingForUiFlush;

#if DEBUG
GCDWebServer *_webServer;
Expand All @@ -108,6 +113,7 @@ - (instancetype)init {
self = [super init];
if (self) {
_eventHandlers = [[NSMutableDictionary alloc] init];
waitingForUiFlush = false;
}
return self;
}
Expand Down Expand Up @@ -274,15 +280,15 @@ - (void)dealloc {

typedef JSGlobalContextRef (^JSContextRefExtractor)();

void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor) {
void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::function<void()> flushUiQueueFn) {
// Make sure the previous JS thread is completely finished before continuing.
static __weak NSThread *s_currentJSThread;
while (s_currentJSThread && !s_currentJSThread.finished) {
[NSThread sleepForTimeInterval:0.1];
}
s_currentJSThread = [NSThread currentThread];

RJSInitializeInContext(jsContextExtractor());
RJSInitializeInContext(jsContextExtractor(), flushUiQueueFn);
}

- (void)setBridge:(RCTBridge *)bridge {
Expand Down Expand Up @@ -310,7 +316,7 @@ - (void)setBridge:(RCTBridge *)bridge {
if (!self || !bridge) {
return;
}

_initializeOnJSThread(^{
// RN < 0.58 has a private method that returns the js context
if ([bridge respondsToSelector:@selector(jsContextRef)]) {
Expand All @@ -325,6 +331,28 @@ - (void)setBridge:(RCTBridge *)bridge {
JSGlobalContextRef ctx_;
};
return static_cast<RealmJSCRuntime*>(bridge.runtime)->ctx_;
}, ^{
// Calling jsCallInvokver->invokeAsync tells React Native to execute the lambda passed
// in on the JS thread, and then flush the internal "microtask queue", which has the
// effect of flushing any pending UI updates.
//
// We call this after we have called into JS from C++, in order to ensure that the RN
// UI updates in response to any changes from Realm. We need to do this as we bypass
// the usual RN bridge mechanism for communicating between C++ and JS, so without doing
// this RN has no way to know that a change has occurred which might require an update
// (see #4389, facebook/react-native#33006).
//
// Calls are debounced using the waitingForUiFlush flag, so if an async flush is already
// pending when another JS to C++ call happens, we don't call invokeAsync again. This works
// because the work is performed before the microtask queue is flushed - see sequence
// diagram at https://bit.ly/3kexhHm. It might be possible to further optimize this,
// e.g. only flush the queue a maximum of once per frame, but this seems reasonable.
if (!waitingForUiFlush) {
waitingForUiFlush = true;
[bridge jsCallInvoker]->invokeAsync([&](){
waitingForUiFlush = false;
});
}
});
} queue:RCTJSThread];
} else { // React Native 0.44 and older
Expand All @@ -341,6 +369,8 @@ - (void)setBridge:(RCTBridge *)bridge {

_initializeOnJSThread(^ {
return RealmReactGetJSGlobalContextForExecutor(executor, true);
}, [&]() {
// jsCallInvoker does not exist on older RN
});
}];
}
Expand Down
5 changes: 4 additions & 1 deletion src/android/jsc_override.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ static JSGlobalContextRef create_context(JSContextGroupRef group, JSClassRef glo
// Clear cache from previous instances.
RJSInvalidateCaches();

RJSInitializeInContext(ctx);
// We pass a no-op lambda for the `flushUiQueue` function, as there's no easy way to
// access the React Native context from here and the UI flush bug (#4389) has not been
// seen on Android. We should be able to do this properly on Hermes.
RJSInitializeInContext(ctx, []() {});
realmContextInjected = true;
return ctx;
}
Expand Down
5 changes: 4 additions & 1 deletion src/jsc/jsc_class.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#pragma once

#include "jsc_types.hpp"
#include "jsc_function.hpp"

#include "js_class.hpp"
#include "js_util.hpp"
Expand All @@ -41,7 +42,7 @@ extern js::Protected<JSObjectRef> FunctionPrototype;
extern js::Protected<JSObjectRef> RealmObjectClassConstructor;
extern js::Protected<JSObjectRef> RealmObjectClassConstructorPrototype;

static inline void jsc_class_init(JSContextRef ctx, JSObjectRef globalObject)
static inline void jsc_class_init(JSContextRef ctx, JSObjectRef globalObject, std::function<void()> flushUiQueue)
{
// handle ReactNative app refresh by reseting the cached constructor values
if (RealmObjectClassConstructor) {
Expand All @@ -63,6 +64,8 @@ static inline void jsc_class_init(JSContextRef ctx, JSObjectRef globalObject)
JSObjectRef globalFunction = jsc::Value::to_object(ctx, value);
value = jsc::Object::get_property(ctx, globalFunction, "prototype");
FunctionPrototype = js::Protected<JSObjectRef>(ctx, Value::to_object(ctx, value));

js::flush_ui_queue = flushUiQueue;
}

template <typename T>
Expand Down
9 changes: 9 additions & 0 deletions src/jsc/jsc_function.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,18 @@
namespace realm {
namespace js {

// Function passed in from the React Native initialisation code to flush the UI microtask queue
extern std::function<void()> flush_ui_queue;

template <>
inline JSValueRef jsc::Function::call(JSContextRef ctx, const JSObjectRef& function, const JSObjectRef& this_object,
size_t argc, const JSValueRef arguments[])
{
JSValueRef exception = nullptr;
JSValueRef result = JSObjectCallAsFunction(ctx, function, this_object, argc, arguments, &exception);

flush_ui_queue();

if (exception) {
throw jsc::Exception(ctx, exception);
}
Expand All @@ -48,6 +54,9 @@ inline JSObjectRef jsc::Function::construct(JSContextRef ctx, const JSObjectRef&
{
JSValueRef exception = nullptr;
JSObjectRef result = JSObjectCallAsConstructor(ctx, function, argc, arguments, &exception);

flush_ui_queue();

if (exception) {
throw jsc::Exception(ctx, exception);
}
Expand Down
8 changes: 6 additions & 2 deletions src/jsc/jsc_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ js::Protected<JSObjectRef> FunctionPrototype;
js::Protected<JSObjectRef> RealmObjectClassConstructor;
js::Protected<JSObjectRef> RealmObjectClassConstructorPrototype;
} // namespace jsc

namespace js {
std::function<void()> flush_ui_queue;
} // namespace js
} // namespace realm

extern "C" {
Expand All @@ -43,13 +47,13 @@ JSObjectRef RJSConstructorCreate(JSContextRef ctx)
return js::RealmClass<Types>::create_constructor(ctx);
}

void RJSInitializeInContext(JSContextRef ctx)
void RJSInitializeInContext(JSContextRef ctx, std::function<void()> flush_ui_queue)
{
static const jsc::String realm_string = "Realm";

JSObjectRef global_object = JSContextGetGlobalObject(ctx);

jsc_class_init(ctx, global_object);
jsc_class_init(ctx, global_object, flush_ui_queue);

JSObjectRef realm_constructor = RJSConstructorCreate(ctx);

Expand Down
3 changes: 2 additions & 1 deletion src/jsc/jsc_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
#pragma once

#include <JavaScriptCore/JSBase.h>
#include <functional>

#ifdef __cplusplus
extern "C" {
#endif

JSObjectRef RJSConstructorCreate(JSContextRef ctx);
void RJSInitializeInContext(JSContextRef ctx);
void RJSInitializeInContext(JSContextRef ctx, std::function<void()> flush_ui_queue);
void RJSInvalidateCaches();

#ifdef __cplusplus
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ RPCServerImpl::RPCServerImpl()
}

m_requests["/create_session"] = [this](const json dict) {
RJSInitializeInContext(m_context);
RJSInitializeInContext(m_context, []() {});

jsc::String realm_string = "Realm";
JSObjectRef realm_constructor =
Expand Down