Skip to content

Conversation

@dividedmind
Copy link
Collaborator

@dividedmind dividedmind commented May 22, 2025

Replace custom recursive directory creation with built-in recursive mkdir in node addon loading

When multiple packaged applications try to start simultaneously, they can encounter race conditions when creating temporary directories for loading native node addons. This manifests as an "EEXIST" error when calling mkdirSync().

The issue occurs in the native module loading code path where pkg needs to extract native addons to a temporary location before they can be loaded via process.dlopen(). The current implementation uses a custom createDirRecursively() function that has a race condition - it checks if a directory exists and then tries to create it, but another process could create the directory between the check and creation.

Node.js has built-in support for recursive directory creation via the recursive: true option in mkdirSync(). This handles race conditions properly - if the directory already exists, it will not throw an error. This is exactly what we need.

@dividedmind dividedmind self-assigned this May 22, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a race condition in temporary directory creation for native node addon loading by replacing a custom recursive directory creation function with the built-in recursive mkdir feature in Node.js.

  • Replace custom createDirRecursively() with fs.mkdirSync(tmpFolder, { recursive: true })
  • Update package.json to apply a patch for pkg version 5.8.1

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
package.json Updated resolutions to include the patched pkg version
.yarn/patches/pkg-npm-5.8.1-db9700609f.patch Replaced the custom recursive directory creation function with fs.mkdirSync with recursive enabled

Replace custom recursive directory creation with built-in recursive mkdir in
node addon loading.

When multiple packaged applications try to start simultaneously, they can
encounter race conditions when creating temporary directories for loading native
node addons. This manifests as an "EEXIST" error when calling `mkdirSync()`.

The issue occurs in the native module loading code path where pkg needs to
extract native addons to a temporary location before they can be loaded via
`process.dlopen()`. The current implementation uses a custom
`createDirRecursively()` function that has a race condition - it checks if a
directory exists and then tries to create it, but another process could create
the directory between the check and creation.

Node.js has built-in support for recursive directory creation via the
`recursive: true` option in `mkdirSync()`. This handles race conditions properly
- if the directory already exists, it will not throw an error. This is exactly
what we need.
@dividedmind dividedmind force-pushed the fix/patch-pkg-eexist branch from 30e1f0f to 987fda0 Compare May 22, 2025 14:22
@dividedmind
Copy link
Collaborator Author

Here's the upstream PR for reference: yao-pkg/pkg#154

@dividedmind dividedmind merged commit 83f236a into main May 22, 2025
26 checks passed
@dividedmind dividedmind deleted the fix/patch-pkg-eexist branch May 22, 2025 14:27
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.

3 participants