Skip to content

Fix preinstall deadlock on Windows by using COREPACK_PACKAGE_MANAGER_LOCATOR (#799)#801

Open
AMDphreak wants to merge 2 commits intonodejs:mainfrom
AMDphreak:fix/preinstall-deadlock-windows
Open

Fix preinstall deadlock on Windows by using COREPACK_PACKAGE_MANAGER_LOCATOR (#799)#801
AMDphreak wants to merge 2 commits intonodejs:mainfrom
AMDphreak:fix/preinstall-deadlock-windows

Conversation

@AMDphreak
Copy link

This PR fixes a deadlock/race condition on Windows where project preinstall scripts that invoke the package manager (e.g., pnpm dlx only-allow pnpm) can crash during corepack use because the global shim is being re-linked or locked by Corepack. This is documented in #799.\n\n### Fix\nIn
unVersion, we now set the COREPACK_PACKAGE_MANAGER_LOCATOR environment variable. In �xecutePackageManagerRequest, we check this variable. If it matches the requested binary, we use it directly to bypass the project specification lookup and shim resolution.\n\n### Why this helps\nThis ensures that child processes sparked by the package manager during lifestyle scripts use the already-resolved version immediately, avoiding any interaction with external shims that might be in a transient or locked state.

…LOCATOR

- Introduce COREPACK_PACKAGE_MANAGER_LOCATOR to ensure sub-invocations use the current binary version.
- Bypass shim/PATH resolution in executePackageManagerRequest if the locator is already set.
- This prevents race conditions where child processes (like preinstall scripts) try to access a shim that is being updated or locked by the main process.
Copy link
Contributor

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

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

As far as I can judge, this PR fails linting and tests. Both would need to be fixed.
It also introduces a new environment variable COREPACK_PACKAGE_MANAGER_LOCATOR which is undocumented in the README.

Again, note that I am not a maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants