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

Update externals stubs atomically #1149

Merged
merged 1 commit into from Mar 4, 2022
Merged

Update externals stubs atomically #1149

merged 1 commit into from Mar 4, 2022

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Mar 3, 2022

Since babel can run in many workers, and since we write the external stubs as a side-effect of discovering that they're needed from within babel, it's possible that a stub is written at the exact moment another worker is reading it. So we need to make sure our writes are atomic.

@ef4
Copy link
Contributor Author

ef4 commented Mar 3, 2022

Ugh, yes, windows doesn't have atomic rename. 😭

Since babel can run in many workers, and since we write the external stubs as a side-effect of discovering that they're needed from within babel, it's possible that a stub is written at the exact moment another worker is reading it. So we need to make sure our writes are atomic.
@ef4 ef4 merged commit 5b923af into main Mar 4, 2022
@ef4 ef4 deleted the externals-race branch March 4, 2022 02:59
} catch (err: any) {
// windows throws EPERM for concurrent access. For us it's not an error
// condition because the other thread is writing the exact same value we
// would have.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about async func here with few retry attempts. I think it’s important to do job, instead of fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we hit EPERM here it's only because another process is already doing the exact same job we were trying to do, so we can move on and let that process take care of it.

As far as I can tell, that's the only way to get EPERM here. Since this is inside our own temp dir, you shouldn't ever have unrelated permission failures in the usual Unix EPERM sense.

@ef4 ef4 added the bug Something isn't working label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants