From 708cff7f4bfa8a90ffc022bc162eee0d7ace0430 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 03:50:32 +0000 Subject: [PATCH 1/2] Convert RemoveChildren from recursive to iterative stack-based Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/bd5ca209-e811-4c49-bec7-5dd3a40d8c57 Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com> --- tools/utils/file_at_fd.go | 117 ++++++++++++++++++++++++-------------- 1 file changed, 74 insertions(+), 43 deletions(-) diff --git a/tools/utils/file_at_fd.go b/tools/utils/file_at_fd.go index 2435ed6c2..909557065 100644 --- a/tools/utils/file_at_fd.go +++ b/tools/utils/file_at_fd.go @@ -230,60 +230,91 @@ func LinkAt(oldparent *os.File, oldname string, newparent *os.File, newname stri return } -// RemoveChildren recursively removes all files and subdirectories -// within the directory pointed to by dirFile. Removes all it can but returns -// the first error, if any. +// RemoveChildren removes all files and subdirectories within the directory +// pointed to by dirFile using an explicit stack instead of recursion. Removes +// all it can but returns the first error, if any. func RemoveChildren(dirFile *os.File) error { var firstErr error - // Rewind directory pointer to ensure we start from the beginning - if _, err := dirFile.Seek(0, io.SeekStart); err != nil { - return err + // Each stack frame is one of two kinds: + // expand: dir != nil – read dir's children, unlink files, push subdirs + // rmdir: dir == nil – rmdir name from parent (a dup'd fd), then close parent + type frame struct { + dir *os.File // non-nil: expand this directory + name string // rmdir: child name to remove from parent + parent *os.File // rmdir: dup'd parent fd (closed after use) + close bool // expand: close dir when done (false only for the root) } - for { - // Read names in small chunks to handle very large directories - entries, err := dirFile.ReadDir(64) - if err != nil { - if errors.Is(err, io.EOF) { + stack := []frame{{dir: dirFile, close: false}} + + for len(stack) > 0 { + f := stack[len(stack)-1] + stack = stack[:len(stack)-1] + + if f.dir == nil { + // rmdir sentinel: the subdirectory is now empty, remove it. + if err := RemoveDirAt(f.parent, f.name); err != nil && firstErr == nil { + firstErr = err + } + f.parent.Close() + continue + } + + // Rewind so we always start from the beginning of the directory. + if _, err := f.dir.Seek(0, io.SeekStart); err != nil { + if firstErr == nil { + firstErr = err + } + if f.close { + f.dir.Close() + } + continue + } + + for { + // Read entries in small chunks to handle very large directories. + entries, err := f.dir.ReadDir(64) + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() { + childFile, openErr := OpenDirAt(f.dir, name) + if openErr != nil { + if firstErr == nil { + firstErr = openErr + } + continue + } + parentDup, dupErr := DupFile(f.dir) + if dupErr != nil { + if firstErr == nil { + firstErr = dupErr + } + childFile.Close() + continue + } + // Push rmdir sentinel first; LIFO ensures the expand frame + // below is processed before this rmdir sentinel. + stack = append(stack, frame{name: name, parent: parentDup}, frame{dir: childFile, close: true}) + } else { + if unlinkErr := UnlinkAt(f.dir, name); unlinkErr != nil && firstErr == nil { + firstErr = unlinkErr + } + } + } + if err != nil { + if !errors.Is(err, io.EOF) && firstErr == nil { + firstErr = &os.PathError{Op: "readdir", Path: f.dir.Name(), Err: err} + } break } - if firstErr == nil { - firstErr = &os.PathError{Op: "readdirnames", Path: dirFile.Name(), Err: err} - } - break } - for _, entry := range entries { - name := entry.Name() - if entry.IsDir() { - // Open subdirectory relative to parent FD - var childFile *os.File - if childFile, err = OpenDirAt(dirFile, name); err != nil { - if firstErr == nil { - firstErr = err - } - continue - } - err = RemoveChildren(childFile) - childFile.Close() - if err == nil { - // Remove the empty subdirectory - if err = RemoveDirAt(dirFile, name); err != nil && firstErr == nil { - firstErr = err - } - } else if firstErr == nil { - firstErr = err - } - childFile.Close() - } else { - // Remove file/symlink - if err = UnlinkAt(dirFile, name); err != nil && firstErr == nil { - firstErr = err - } - } + if f.close { + f.dir.Close() } } + _, _ = dirFile.Seek(0, io.SeekStart) return firstErr } From 7757d160bceb6ef5101da192388af70302ceab84 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 04:01:02 +0000 Subject: [PATCH 2/2] RemoveChildren: use RefCountedFile instead of DupFile, skip seek for child dirs Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/e648c02e-d4cc-4f6c-8188-17941e8a1dd8 Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com> --- tools/utils/file_at_fd.go | 62 ++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/tools/utils/file_at_fd.go b/tools/utils/file_at_fd.go index 909557065..7e3033f5d 100644 --- a/tools/utils/file_at_fd.go +++ b/tools/utils/file_at_fd.go @@ -238,15 +238,23 @@ func RemoveChildren(dirFile *os.File) error { // Each stack frame is one of two kinds: // expand: dir != nil – read dir's children, unlink files, push subdirs - // rmdir: dir == nil – rmdir name from parent (a dup'd fd), then close parent + // rmdir: dir == nil – rmdir name from parent, then Unref parent type frame struct { - dir *os.File // non-nil: expand this directory - name string // rmdir: child name to remove from parent - parent *os.File // rmdir: dup'd parent fd (closed after use) - close bool // expand: close dir when done (false only for the root) + dir *RefCountedFile // non-nil: expand this directory (Unref when done) + name string // rmdir sentinel: child name to remove from parent + parent *RefCountedFile // rmdir sentinel: ref to parent dir (Unref after rmdir) } - stack := []frame{{dir: dirFile, close: false}} + // Only the root directory needs rewinding; it was passed in from outside + // and may have been read before. Child dirs are freshly opened by us. + if _, err := dirFile.Seek(0, io.SeekStart); err != nil { + return err + } + rcRoot := NewRefCountedFile(dirFile) + // Extra ref so the expand frame's Unref doesn't close the caller's fd. + rcRoot.NewRef() + + stack := []frame{{dir: rcRoot}} for len(stack) > 0 { f := stack[len(stack)-1] @@ -254,65 +262,45 @@ func RemoveChildren(dirFile *os.File) error { if f.dir == nil { // rmdir sentinel: the subdirectory is now empty, remove it. - if err := RemoveDirAt(f.parent, f.name); err != nil && firstErr == nil { + if err := RemoveDirAt(f.parent.File(), f.name); err != nil && firstErr == nil { firstErr = err } - f.parent.Close() - continue - } - - // Rewind so we always start from the beginning of the directory. - if _, err := f.dir.Seek(0, io.SeekStart); err != nil { - if firstErr == nil { - firstErr = err - } - if f.close { - f.dir.Close() - } + f.parent.Unref() continue } for { // Read entries in small chunks to handle very large directories. - entries, err := f.dir.ReadDir(64) + entries, err := f.dir.File().ReadDir(64) for _, entry := range entries { name := entry.Name() if entry.IsDir() { - childFile, openErr := OpenDirAt(f.dir, name) + childFile, openErr := OpenDirAt(f.dir.File(), name) if openErr != nil { if firstErr == nil { firstErr = openErr } continue } - parentDup, dupErr := DupFile(f.dir) - if dupErr != nil { - if firstErr == nil { - firstErr = dupErr - } - childFile.Close() - continue - } - // Push rmdir sentinel first; LIFO ensures the expand frame - // below is processed before this rmdir sentinel. - stack = append(stack, frame{name: name, parent: parentDup}, frame{dir: childFile, close: true}) + // Push rmdir sentinel first; LIFO ensures the child expand + // frame is processed before this rmdir sentinel. + // f.dir.NewRef() adds a ref to the parent for the sentinel. + stack = append(stack, frame{name: name, parent: f.dir.NewRef()}, frame{dir: NewRefCountedFile(childFile)}) } else { - if unlinkErr := UnlinkAt(f.dir, name); unlinkErr != nil && firstErr == nil { + if unlinkErr := UnlinkAt(f.dir.File(), name); unlinkErr != nil && firstErr == nil { firstErr = unlinkErr } } } if err != nil { if !errors.Is(err, io.EOF) && firstErr == nil { - firstErr = &os.PathError{Op: "readdir", Path: f.dir.Name(), Err: err} + firstErr = &os.PathError{Op: "readdir", Path: f.dir.File().Name(), Err: err} } break } } - if f.close { - f.dir.Close() - } + f.dir.Unref() } _, _ = dirFile.Seek(0, io.SeekStart)