Skip to content

Fix node:fs cp to handle symlinks when copying into subdirectories - #7098

Open
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/fs-symlink-copy
Open

Fix node:fs cp to handle symlinks when copying into subdirectories#7098
ryanking13 wants to merge 1 commit into
mainfrom
gyeongjae/fs-symlink-copy

Conversation

@ryanking13

Copy link
Copy Markdown
Contributor

Fixes node:fs cp functions to handle subdirectories that contain symlink properly.

When the dest directory contains a symlink to the src directory, it could free up the memory of the src directory during the copy.

This fixes it by holding a refcount of all the entries in the src directory before performing the copy.

@ryanking13
ryanking13 requested a review from jasnell August 24, 2026 07:21
@ryanking13
ryanking13 requested review from a team as code owners August 24, 2026 07:21
Comment thread src/workerd/api/node/tests/fs-cp-into-subdirectory-test.js
@ask-bonk

ask-bonk Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

I'm Bonk, and I've done a quick review of your PR.

PR #7098 snapshots source directory entries before recursive node:fs copies.

  1. [P2] The regression test does not verify entries after the symlinked child are copied. Posted an inline suggestion to assert a.txt, b.txt, and c.txt reach the destination.

github run

@ryanking13
ryanking13 force-pushed the gyeongjae/fs-symlink-copy branch 2 times, most recently from b7ba948 to 3f277db Compare August 24, 2026 07:56
@ryanking13
ryanking13 force-pushed the gyeongjae/fs-symlink-copy branch from 3f277db to dd648d8 Compare August 25, 2026 02:57
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