Skip to content

Commit

Permalink
fix app crashing when reloads overlap (#46416)
Browse files Browse the repository at this point in the history
Summary:
Regarding the [issue](#44755) where the app sometimes crashes due to race condition when two reloads overlap in unfortunate way. This PR fixes it in some way by introducing throttling on reload command. For now I set it to 700ms as I was still able to reproduce it on 500-550ms for provided repro in the issue. The problem may still happen for bigger apps where reload may take more time to finish.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[GENERAL] [FIXED] - throttle reload command

Pull Request resolved: #46416

Test Plan: I've tested on provided repro and a smaller app trying to brake it.

Reviewed By: huntie

Differential Revision: D62847076

Pulled By: cipolleschi

fbshipit-source-id: 6471f792d6b692e87e3e98a699443a88c6ef43cd
  • Loading branch information
coado authored and facebook-github-bot committed Sep 18, 2024
1 parent 7de3111 commit 42bad68
Showing 1 changed file with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@ import fetch from 'node-fetch';

const CTRL_C = '\u0003';
const CTRL_D = '\u0004';
const RELOAD_TIMEOUT = 500;

const throttle = (callback: () => void, timeout: number) => {
let previousCallTimestamp = 0;
return () => {
const currentCallTimestamp = new Date().getTime();
if (currentCallTimestamp - previousCallTimestamp > timeout) {
previousCallTimestamp = currentCallTimestamp;
callback();
}
};
};

export default function attachKeyHandlers({
cliConfig,
Expand All @@ -41,11 +53,15 @@ export default function attachKeyHandlers({
env: {FORCE_COLOR: chalk.supportsColor ? 'true' : 'false'},
};

const reload = throttle(() => {
logger.info('Reloading connected app(s)...');
messageSocket.broadcast('reload', null);
}, RELOAD_TIMEOUT);

const onPress = async (key: string) => {
switch (key.toLowerCase()) {
case 'r':
logger.info('Reloading connected app(s)...');
messageSocket.broadcast('reload', null);
reload();
break;
case 'd':
logger.info('Opening Dev Menu...');
Expand Down

0 comments on commit 42bad68

Please sign in to comment.