Skip to content

Commit f35e729

Browse files
Fix race condition in FileSystem.create*Link helpers (#5655)
* Initial plan * Fix race condition in FileSystem.create*Link helpers by factoring isExistError handler into dedicated helpers Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dmichon-msft <26827560+dmichon-msft@users.noreply.github.com>
1 parent 802cdb3 commit f35e729

2 files changed

Lines changed: 74 additions & 28 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/node-core-library",
5+
"comment": "Fix race condition in FileSystem.create*Link helpers: EEXIST errors that occur after ensureFolder/ensureFolderAsync are now handled consistently with the initial EEXIST handling.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@rushstack/node-core-library"
10+
}

libraries/node-core-library/src/FileSystem.ts

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,25 +1518,33 @@ export class FileSystem {
15181518
);
15191519
}
15201520

1521+
private static _handleLinkExistError(
1522+
linkFn: () => void,
1523+
options: IInternalFileSystemCreateLinkOptions,
1524+
error: Error
1525+
): void {
1526+
switch (options.alreadyExistsBehavior) {
1527+
case AlreadyExistsBehavior.Ignore:
1528+
break;
1529+
case AlreadyExistsBehavior.Overwrite:
1530+
// fsx.linkSync does not allow overwriting so we must manually delete. If it's
1531+
// a folder, it will throw an error.
1532+
this.deleteFile(options.newLinkPath);
1533+
linkFn();
1534+
break;
1535+
case AlreadyExistsBehavior.Error:
1536+
default:
1537+
throw error;
1538+
}
1539+
}
1540+
15211541
private static _handleLink(linkFn: () => void, options: IInternalFileSystemCreateLinkOptions): void {
15221542
try {
15231543
linkFn();
15241544
} catch (error) {
15251545
if (FileSystem.isExistError(error as Error)) {
15261546
// Link exists, handle it
1527-
switch (options.alreadyExistsBehavior) {
1528-
case AlreadyExistsBehavior.Ignore:
1529-
break;
1530-
case AlreadyExistsBehavior.Overwrite:
1531-
// fsx.linkSync does not allow overwriting so we must manually delete. If it's
1532-
// a folder, it will throw an error.
1533-
this.deleteFile(options.newLinkPath);
1534-
linkFn();
1535-
break;
1536-
case AlreadyExistsBehavior.Error:
1537-
default:
1538-
throw error;
1539-
}
1547+
FileSystem._handleLinkExistError(linkFn, options, error as Error);
15401548
} else {
15411549
// When attempting to create a link in a directory that does not exist, an ENOENT
15421550
// or ENOTDIR error is thrown, so we should ensure the directory exists before
@@ -1547,14 +1555,44 @@ export class FileSystem {
15471555
(!options.linkTargetMustExist || FileSystem.exists(options.linkTargetPath))
15481556
) {
15491557
this.ensureFolder(nodeJsPath.dirname(options.newLinkPath));
1550-
linkFn();
1558+
try {
1559+
linkFn();
1560+
} catch (retryError) {
1561+
if (FileSystem.isExistError(retryError as Error)) {
1562+
// Another concurrent process may have created the link between the ensureFolder
1563+
// call and the retry; handle it the same way as the initial exist error.
1564+
FileSystem._handleLinkExistError(linkFn, options, retryError as Error);
1565+
} else {
1566+
throw retryError;
1567+
}
1568+
}
15511569
} else {
15521570
throw error;
15531571
}
15541572
}
15551573
}
15561574
}
15571575

1576+
private static async _handleLinkExistErrorAsync(
1577+
linkFn: () => Promise<void>,
1578+
options: IInternalFileSystemCreateLinkOptions,
1579+
error: Error
1580+
): Promise<void> {
1581+
switch (options.alreadyExistsBehavior) {
1582+
case AlreadyExistsBehavior.Ignore:
1583+
break;
1584+
case AlreadyExistsBehavior.Overwrite:
1585+
// fsx.linkSync does not allow overwriting so we must manually delete. If it's
1586+
// a folder, it will throw an error.
1587+
await this.deleteFileAsync(options.newLinkPath);
1588+
await linkFn();
1589+
break;
1590+
case AlreadyExistsBehavior.Error:
1591+
default:
1592+
throw error;
1593+
}
1594+
}
1595+
15581596
private static async _handleLinkAsync(
15591597
linkFn: () => Promise<void>,
15601598
options: IInternalFileSystemCreateLinkOptions
@@ -1564,19 +1602,7 @@ export class FileSystem {
15641602
} catch (error) {
15651603
if (FileSystem.isExistError(error as Error)) {
15661604
// Link exists, handle it
1567-
switch (options.alreadyExistsBehavior) {
1568-
case AlreadyExistsBehavior.Ignore:
1569-
break;
1570-
case AlreadyExistsBehavior.Overwrite:
1571-
// fsx.linkSync does not allow overwriting so we must manually delete. If it's
1572-
// a folder, it will throw an error.
1573-
await this.deleteFileAsync(options.newLinkPath);
1574-
await linkFn();
1575-
break;
1576-
case AlreadyExistsBehavior.Error:
1577-
default:
1578-
throw error;
1579-
}
1605+
await FileSystem._handleLinkExistErrorAsync(linkFn, options, error as Error);
15801606
} else {
15811607
// When attempting to create a link in a directory that does not exist, an ENOENT
15821608
// or ENOTDIR error is thrown, so we should ensure the directory exists before
@@ -1587,7 +1613,17 @@ export class FileSystem {
15871613
(!options.linkTargetMustExist || (await FileSystem.existsAsync(options.linkTargetPath)))
15881614
) {
15891615
await this.ensureFolderAsync(nodeJsPath.dirname(options.newLinkPath));
1590-
await linkFn();
1616+
try {
1617+
await linkFn();
1618+
} catch (retryError) {
1619+
if (FileSystem.isExistError(retryError as Error)) {
1620+
// Another concurrent process may have created the link between the ensureFolderAsync
1621+
// call and the retry; handle it the same way as the initial exist error.
1622+
await FileSystem._handleLinkExistErrorAsync(linkFn, options, retryError as Error);
1623+
} else {
1624+
throw retryError;
1625+
}
1626+
}
15911627
} else {
15921628
throw error;
15931629
}

0 commit comments

Comments
 (0)