mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-13 20:39:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address CodeRabbit review: Add cleanup and race handling

Browse Source 
- Add errdefer cleanup for temp directories on failure
- Handle race condition when destination already exists
- Set moved flag to prevent cleanup after successful rename
- Log absolute paths instead of relative in error messages
- Best-effort cleanup when another process wins the race

Co-Authored-By: CodeRabbit AI <noreply@coderabbit.ai>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2025-10-13 16:52:15 +00:00
parent c61f11242b
commit 63db5023dd
1 changed files with 20 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
src/install/repository.zig
View File

@@ -518,6 +518,8 @@ pub const Repository = extern struct {
const tmpname = try FileSystem.tmpname(folder_name[0..@min(folder_name.len, 32)], &tmpname_buf, bun.fastRandom());
const temp_target = try bun.getFdPath(.fromStdDir(temp_dir), &temp_path_buf);
const temp_target_full = Path.joinAbsString(temp_target, &.{tmpname}, .auto);
var moved = false;
errdefer if (!moved) temp_dir.deleteTree(tmpname) catch {};
_ = exec(allocator, env, &[_]string{
"git",
@@ -548,15 +550,22 @@ pub const Repository = extern struct {
folder_name,
.{ .move_fallback = true },
).asErr()) |err| {
// If destination exists, another process likely completed the work
if (cache_dir.openDirZ(folder_name, .{})) |dir2| {
// Best-effort cleanup of our temp path
temp_dir.deleteTree(tmpname) catch {};
break :clone dir2;
} else |_| {}
log.addErrorFmt(
null,
logger.Loc.Empty,
allocator,
"moving \"{s}\" to cache dir failed: {}\n From: {s}\n To: {s}",
.{ name, err, tmpname, folder_name },
.{ name, err, temp_target_full, folder_name },
) catch unreachable;
return error.InstallFailed;
}
moved = true;
break :clone try cache_dir.openDirZ(folder_name, .{});
};
@@ -618,6 +627,8 @@ pub const Repository = extern struct {
const tmpname = try FileSystem.tmpname(folder_name[0..@min(folder_name.len, 32)], &tmpname_buf, bun.fastRandom());
const temp_target = try bun.getFdPath(.fromStdDir(temp_dir), &temp_path_buf);
const temp_target_full = Path.joinAbsString(temp_target, &.{tmpname}, .auto);
var moved = false;
errdefer if (!moved) temp_dir.deleteTree(tmpname) catch {};
_ = exec(allocator, env, &[_]string{
"git",
@@ -669,15 +680,22 @@ pub const Repository = extern struct {
folder_name,
.{ .move_fallback = true },
).asErr()) |err| {
// If destination exists, another process likely completed the work
if (bun.openDir(cache_dir, folder_name)) |_| {
// Best-effort cleanup of our temp path
temp_dir.deleteTree(tmpname) catch {};
break :brk try bun.openDir(cache_dir, folder_name);
} else |_| {}
log.addErrorFmt(
null,
logger.Loc.Empty,
allocator,
"moving \"{s}\" to cache dir failed: {}\n From: {s}\n To: {s}",
.{ name, err, tmpname, folder_name },
.{ name, err, temp_target_full, folder_name },
) catch unreachable;
return error.InstallFailed;
}
moved = true;
break :brk try bun.openDir(cache_dir, folder_name);
};