Permalink
Browse files

Allow native modules to customize the main-thread init behaviour

Summary:
We've simplified a lot of the conditions for eager of the module init so now we can introduce a final switch to allow modules to opt-out (and in the future opt-in if they still require the behaviour).

We now require you to be explicit about the intended behaviour and implement the `+ (BOOL)requiresMainQueueSetup` method on your module. When you return YES from this method, it tells the bridge the module needs to be created on the main thread (and to avoid deadlocks, we do so eagerly during bridge startup). When you return NO, the native module will be initialised when it's first accessed from JS.

The current behaviour is maintained but a warning is emitted until the new API is adopted.

Reviewed By: fkgozali

Differential Revision: D5527788

fbshipit-source-id: 56d38f81e58cf950547b9780e89bfac4667eeaaa
  • Loading branch information...
javache authored and facebook-github-bot committed Aug 7, 2017
1 parent 1cd276a commit d42ccca2e1451133504d35bb9e02adea25e5deaa
Showing with 57 additions and 21 deletions.
  1. +21 −6 React/Base/RCTBridgeModule.h
  2. +31 −15 React/Base/RCTModuleData.mm
  3. +5 −0 React/CxxModule/RCTCxxModule.mm
@@ -263,6 +263,19 @@ RCT_EXTERN void RCTRegisterModule(Class); \
return &config; \
}
/**
* Most modules can be used from any thread. All of the modules exported non-sync method will be called on its
* methodQueue, and the module will be constructed lazily when its first invoked. Some modules have main need to access

This comment has been minimized.

Show comment
Hide comment
@koenpunt

koenpunt Dec 13, 2017

Contributor

"main need" ?

@koenpunt

koenpunt Dec 13, 2017

Contributor

"main need" ?

* information that's main queue only (e.g. most UIKit classes). Since we don't want to dispatch synchronously to the
* main thread to this safely, we construct these moduels and export their constants ahead-of-time.

This comment has been minimized.

Show comment
Hide comment
@koenpunt

koenpunt Dec 13, 2017

Contributor

"to this safely"? "moduels"?

@koenpunt

koenpunt Dec 13, 2017

Contributor

"to this safely"? "moduels"?

*
* Note that when set to false, the module constructor will be called from any thread.
*
* This requirement is currently inferred by checking if the module has a custom initializer or if there's exported
* constants. In the future, we'll stop automatically inferring this and instead only rely on this method.
*/

This comment has been minimized.

Show comment
Hide comment
@koenpunt

koenpunt Dec 13, 2017

Contributor

There are some typos here that makes it hard to understand. Also the sentences are not very clear IMO.
I'll highlight them above

@koenpunt

koenpunt Dec 13, 2017

Contributor

There are some typos here that makes it hard to understand. Also the sentences are not very clear IMO.
I'll highlight them above

This comment has been minimized.

Show comment
Hide comment
@koenpunt

koenpunt Dec 13, 2017

Contributor

Then also, do I need to return YES from requiresMainQueueSetup when I return dispatch_get_main_queue() from methodQueue? Or only if the init and/or constantsToExport is used?

@koenpunt

koenpunt Dec 13, 2017

Contributor

Then also, do I need to return YES from requiresMainQueueSetup when I return dispatch_get_main_queue() from methodQueue? Or only if the init and/or constantsToExport is used?

This comment has been minimized.

Show comment
Hide comment
@koenpunt

koenpunt Dec 13, 2017

Contributor

(I expect the latter)

@koenpunt

koenpunt Dec 13, 2017

Contributor

(I expect the latter)

+ (BOOL)requiresMainQueueSetup;
/**
* Injects methods into JS. Entries in this array are used in addition to any
* methods defined using the macros above. This method is called only once,
@@ -271,13 +284,15 @@ RCT_EXTERN void RCTRegisterModule(Class); \
- (NSArray<id<RCTBridgeMethod>> *)methodsToExport;
/**
* Injects constants into JS. These constants are made accessible via
* NativeModules.ModuleName.X. It is only called once for the lifetime of the
* bridge, so it is not suitable for returning dynamic values, but may be used
* for long-lived values such as session keys, that are regenerated only as
* part of a reload of the entire React application.
* Injects constants into JS. These constants are made accessible via NativeModules.ModuleName.X. It is only called once
* for the lifetime of the bridge, so it is not suitable for returning dynamic values, but may be used for long-lived
* values such as session keys, that are regenerated only as part of a reload of the entire React application.
*
* If you implement this method and do not implement `requiresMainThreadSetup`, you will trigger deprecated logic
* that eagerly initializes your module on bridge startup. In the future, this behaviour will be changed to default
* to initializing lazily, and even modules with constants will be initialized lazily.
*/
- (NSDictionary<NSString *, id> *)constantsToExport;
- (NSDictionary *)constantsToExport;
/**
* Notifies the module that a batch of JS method invocations has just completed.
@@ -38,21 +38,37 @@ - (void)setUp
_implementsBatchDidComplete = [_moduleClass instancesRespondToSelector:@selector(batchDidComplete)];
_implementsPartialBatchDidFlush = [_moduleClass instancesRespondToSelector:@selector(partialBatchDidFlush)];
static IMP objectInitMethod;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
objectInitMethod = [NSObject instanceMethodForSelector:@selector(init)];
});
// If a module overrides `constantsToExport` then we must assume that it
// must be called on the main thread, because it may need to access UIKit.
// If a module overrides `constantsToExport` and doesn't implement `requiresMainQueueSetup`, then we must assume
// that it must be called on the main thread, because it may need to access UIKit.
_hasConstantsToExport = [_moduleClass instancesRespondToSelector:@selector(constantsToExport)];
// If a module overrides `init` then we must assume that it expects to be
// initialized on the main thread, because it may need to access UIKit.
const BOOL hasCustomInit = !_instance && [_moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod;
const BOOL implementsRequireMainQueueSetup = [_moduleClass respondsToSelector:@selector(requiresMainQueueSetup)];
if (implementsRequireMainQueueSetup) {
_requiresMainQueueSetup = [_moduleClass requiresMainQueueSetup];
} else {
static IMP objectInitMethod;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
objectInitMethod = [NSObject instanceMethodForSelector:@selector(init)];
});
// If a module overrides `init` then we must assume that it expects to be
// initialized on the main thread, because it may need to access UIKit.
const BOOL hasCustomInit = !_instance && [_moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod;
_requiresMainQueueSetup = _hasConstantsToExport || hasCustomInit;
_requiresMainQueueSetup = _hasConstantsToExport || hasCustomInit;
if (_requiresMainQueueSetup) {
const char *methodName = "";
if (_hasConstantsToExport) {
methodName = "constantsToExport";
} else if (hasCustomInit) {
methodName = "init";
}
RCTLogWarn(@"Module %@ requires main queue setup since it overrides `%s` but doesn't implement "
"`requiresMainQueueSetup. In a future release React Native will default to initializing all native modules "
"on a background thread unless explicitly opted-out of.", _moduleClass, methodName);
}
}
}
- (instancetype)initWithModuleClass:(Class)moduleClass
@@ -291,16 +307,16 @@ - (void)gatherConstants
if (_hasConstantsToExport && !_constantsToExport) {
RCT_PROFILE_BEGIN_EVENT(RCTProfileTagAlways, ([NSString stringWithFormat:@"[RCTModuleData gatherConstants] %@", _moduleClass]), nil);
(void)[self instance];
if (!_requiresMainQueueSetup) {
_constantsToExport = [_instance constantsToExport] ?: @{};
} else {
if (_requiresMainQueueSetup) {
if (!RCTIsMainQueue()) {
RCTLogWarn(@"Required dispatch_sync to load constants for %@. This may lead to deadlocks", _moduleClass);
}
RCTUnsafeExecuteOnMainQueueSync(^{
self->_constantsToExport = [self->_instance constantsToExport] ?: @{};
});
} else {
_constantsToExport = [_instance constantsToExport] ?: @{};
}
RCT_PROFILE_END_EVENT(RCTProfileTagAlways, @"");
}
@@ -28,6 +28,11 @@ + (NSString *)moduleName
return @"";
}
+ (BOOL)requiresMainQueueSetup
{
return NO;
}
- (void)lazyInit
{
if (!_module) {

0 comments on commit d42ccca

Please sign in to comment.