Fix a race condition in @babel/core#14843
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52713/ |
| const _plugins = yield* createPluginDescriptors( | ||
| options.plugins || [], | ||
| dirname, | ||
| alias, | ||
| ); | ||
| // check plugins again to avoid race conditions | ||
| plugins ??= _plugins; |
There was a problem hiding this comment.
Instead of just ignoring the result of the second createPluginDescriptors call, can we avoid calling it twice in the first place? Something like this:
let pluginsGen;
return {
options: optionsWithResolvedBrowserslistConfigFile(options, dirname),
*plugins() {
pluginsGen ??= createPluginDescriptors(
options.plugins || [],
dirname,
alias,
);
return yield* pluginsGen;
}
};There was a problem hiding this comment.
If we cache the iterator instead, test will fail because the finished iterator always return undefined: https://github.com/JLHwung/babel/runs/7819160996?check_suite_focus=true
There was a problem hiding this comment.
I pushed a commit to do the caching, so that now:
- The first call runs
createPluginDescriptors, and after a while returns it - The second call waits for the result of the first call, and returns it
Instead of
- The first call runs
createPluginDescriptors, and returns the result of thecreatePluginDescriptorsthat finishes first - The second call runs
createPluginDescriptors, and returns the result of thecreatePluginDescriptorsthat finishes first
This avoids resolving the same plugins/presets multiple times, which is a costly operation because it involves the filesystem.
|
Unfortunately, this test does not reflect the real problem. https://github.com/babel/babel/runs/7775496327?check_suite_focus=true |
|
It's probably because we build Babel using an older version of itself 🤔 This PR still fixes the issue shown in the test, we can potentially wait to revert #14806 until after the new release. |
There was a problem hiding this comment.
Makes sense! Let's merge it first.
For #14806 I think it's good to keep it that way, only a few of us will see these logs anyway.😄
This PR fixes a race condition in
createUncachedDescriptors: After we asynchronously initialize plugins/presets descriptors, we don't check again whether it has been initialized in another task. So we assign different descriptors to config item and the plugins/presets are loaded more than once.I recommend to review this PR by commits.
This PR also reverts #14806 per Nicolò's comment on that PR.