-
Notifications
You must be signed in to change notification settings - Fork 15
fix(utils): prevent race condition in parallel plugin runs #1222
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
base: main
Are you sure you want to change the base?
fix(utils): prevent race condition in parallel plugin runs #1222
Conversation
When running the same plugin for multiple projects in parallel (e.g., via Nx targets), the timestamp-based directory naming could collide if two processes started within the same millisecond. This caused one process to delete the output directory while another was still using it, leading to failures. The fix adds process.pid and a random suffix to the directory name, ensuring uniqueness even when multiple processes run simultaneously.
|
View your CI Pipeline Execution ↗ for commit 4e24f73
☁️ Nx Cloud last updated this comment at |
@code-pushup/ci
@code-pushup/cli
@code-pushup/core
@code-pushup/create-cli
@code-pushup/models
@code-pushup/nx-plugin
@code-pushup/axe-plugin
@code-pushup/coverage-plugin
@code-pushup/eslint-plugin
@code-pushup/js-packages-plugin
@code-pushup/jsdocs-plugin
@code-pushup/lighthouse-plugin
@code-pushup/typescript-plugin
@code-pushup/utils
commit: |
Code PushUp😟 Code PushUp report has regressed – compared current commit 1d7ffda with previous commit 1d7ffda. 🏷️ Categories👎 1 group regressed, 👎 5 audits regressed, 11 audits changed without impacting score🗃️ Groups
33 other groups are unchanged. 🛡️ Audits663 other audits are unchanged. |
BioPhoton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a suggestion
| // Use timestamp + process ID + random suffix to ensure uniqueness | ||
| // This prevents race conditions when running the same plugin for multiple projects in parallel | ||
| // eslint-disable-next-line @typescript-eslint/no-magic-numbers | ||
| const uniqueId = `${Date.now()}-${process.pid}-${Math.random().toString(36).slice(2, 8)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const uniqueId = `${Date.now()}-${process.pid}-${Math.random().toString(36).slice(2, 8)}`; | |
| const uniqueId = `${(performance.timeOrigin+performance.now())*10}-${process.pid}-${threadId}`; |
- We can get an additional digit from
perf_hookswhich is also monotonic:(performance.timeOrigin+performance.now())*10 - Processes can have multiple threads
threadIdis0by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why there is a random suffix. Imo, process + suffix is good enough, but I'll let the rest of the team confirm that
Summary
Fixes a race condition when running the same plugin for multiple projects in parallel (e.g., via Nx targets).
Problem
When two processes started within the same millisecond, the timestamp-based directory naming in
createRunnerFileswould collide:This caused one process to delete the output directory (in
executeRunnerConfig) while another was still using it, leading to failures.Solution
Added
process.pidand a random suffix to the directory name to ensure uniqueness:This ensures that even when multiple processes run simultaneously, each gets its own unique working directory.