From 97b6829b830d26da62ca014ae83b22b40b7c5379 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Wed, 20 Sep 2023 15:07:21 -0700 Subject: [PATCH] Ensure systrace events are always stopped (#39561) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39561 Trying to investigate an issue where Systrace events aren't properly ended. For correctness, we should make sure to always end markers that we started. Reviewed By: sammy-SC Differential Revision: D49413689 fbshipit-source-id: e48e3aef20602e0af9deb924244cdfaf614d11f2 --- .../Libraries/BatchedBridge/MessageQueue.js | 81 ++++++++++--------- .../__tests__/MessageQueue-test.js | 3 +- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/packages/react-native/Libraries/BatchedBridge/MessageQueue.js b/packages/react-native/Libraries/BatchedBridge/MessageQueue.js index 932d309fc1bd0a..07481e543fa854 100644 --- a/packages/react-native/Libraries/BatchedBridge/MessageQueue.js +++ b/packages/react-native/Libraries/BatchedBridge/MessageQueue.js @@ -388,10 +388,13 @@ class MessageQueue { __callReactNativeMicrotasks() { Systrace.beginEvent('JSTimers.callReactNativeMicrotasks()'); - if (this._reactNativeMicrotasksCallback != null) { - this._reactNativeMicrotasksCallback(); + try { + if (this._reactNativeMicrotasksCallback != null) { + this._reactNativeMicrotasksCallback(); + } + } finally { + Systrace.endEvent(); } - Systrace.endEvent(); } __callFunction(module: string, method: string, args: mixed[]): void { @@ -402,31 +405,35 @@ class MessageQueue { } else { Systrace.beginEvent(`${module}.${method}(...)`); } - if (this.__spy) { - this.__spy({type: TO_JS, module, method, args}); - } - const moduleMethods = this.getCallableModule(module); - if (!moduleMethods) { - const callableModuleNames = Object.keys(this._lazyCallableModules); - const n = callableModuleNames.length; - const callableModuleNameList = callableModuleNames.join(', '); - - // TODO(T122225939): Remove after investigation: Why are we getting to this line in bridgeless mode? - const isBridgelessMode = global.RN$Bridgeless === true ? 'true' : 'false'; - invariant( - false, - `Failed to call into JavaScript module method ${module}.${method}(). Module has not been registered as callable. Bridgeless Mode: ${isBridgelessMode}. Registered callable JavaScript modules (n = ${n}): ${callableModuleNameList}. - A frequent cause of the error is that the application entry file path is incorrect. This can also happen when the JS bundle is corrupt or there is an early initialization error when loading React Native.`, - ); - } - if (!moduleMethods[method]) { - invariant( - false, - `Failed to call into JavaScript module method ${module}.${method}(). Module exists, but the method is undefined.`, - ); + try { + if (this.__spy) { + this.__spy({type: TO_JS, module, method, args}); + } + const moduleMethods = this.getCallableModule(module); + if (!moduleMethods) { + const callableModuleNames = Object.keys(this._lazyCallableModules); + const n = callableModuleNames.length; + const callableModuleNameList = callableModuleNames.join(', '); + + // TODO(T122225939): Remove after investigation: Why are we getting to this line in bridgeless mode? + const isBridgelessMode = + global.RN$Bridgeless === true ? 'true' : 'false'; + invariant( + false, + `Failed to call into JavaScript module method ${module}.${method}(). Module has not been registered as callable. Bridgeless Mode: ${isBridgelessMode}. Registered callable JavaScript modules (n = ${n}): ${callableModuleNameList}. + A frequent cause of the error is that the application entry file path is incorrect. This can also happen when the JS bundle is corrupt or there is an early initialization error when loading React Native.`, + ); + } + if (!moduleMethods[method]) { + invariant( + false, + `Failed to call into JavaScript module method ${module}.${method}(). Module exists, but the method is undefined.`, + ); + } + moduleMethods[method].apply(moduleMethods, args); + } finally { + Systrace.endEvent(); } - moduleMethods[method].apply(moduleMethods, args); - Systrace.endEvent(); } __invokeCallback(cbID: number, args: mixed[]): void { @@ -465,16 +472,18 @@ class MessageQueue { ); } - if (!callback) { - return; - } - - this._successCallbacks.delete(callID); - this._failureCallbacks.delete(callID); - callback(...args); + try { + if (!callback) { + return; + } - if (__DEV__) { - Systrace.endEvent(); + this._successCallbacks.delete(callID); + this._failureCallbacks.delete(callID); + callback(...args); + } finally { + if (__DEV__) { + Systrace.endEvent(); + } } } } diff --git a/packages/react-native/Libraries/BatchedBridge/__tests__/MessageQueue-test.js b/packages/react-native/Libraries/BatchedBridge/__tests__/MessageQueue-test.js index 46304de89ad7ac..735b08338b3ac1 100644 --- a/packages/react-native/Libraries/BatchedBridge/__tests__/MessageQueue-test.js +++ b/packages/react-native/Libraries/BatchedBridge/__tests__/MessageQueue-test.js @@ -108,8 +108,7 @@ describe('MessageQueue', function () { const unknownModule = 'UnknownModule', unknownMethod = 'UnknownMethod'; expect(() => queue.__callFunction(unknownModule, unknownMethod)).toThrow( - `Failed to call into JavaScript module method ${unknownModule}.${unknownMethod}(). Module has not been registered as callable. Bridgeless Mode: false. Registered callable JavaScript modules (n = 1): MessageQueueTestModule. - A frequent cause of the error is that the application entry file path is incorrect. This can also happen when the JS bundle is corrupt or there is an early initialization error when loading React Native.`, + `Failed to call into JavaScript module method ${unknownModule}.${unknownMethod}()`, ); });