cockroachdb / cockroachdb/cockroach

cli: file descriptor leak in tsdump upload

Open
#161,401 1 comment 0 reactions 0 assignees View on GitHub
C-cleanup O-AI-Review T-supportability
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Summary

In `pkg/cli/tsdump_upload.go`, the `datadogWriter.upload()` function has a file descriptor leak. The file handle opened by `getFileReader()` is never closed before the function returns.

## Details

The issue is in the `upload()` function (lines 715-960):

1. A file reader is opened at line 716: `f, err := getFileReader(fileName)`
2. If metadata reading succeeds (lines 734-736), this file handle is used for the rest of the function but **never closed**
3. If metadata reading fails, the first file handle is properly closed (lines 740-744), but the replacement file reader opened at line 745 is also **never closed**

The function ends with `close(ch); return nil` without closing the file handle `f`.

## Impact

- File descriptors are a limited system resource
- In environments with repeated tsdump uploads or long-running processes, this could lead to file descriptor exhaustion
- Severity is low since this is a CLI tool typically run infrequently, but it should still be fixed for correctness

## Suggested Fix

Add a deferred close after opening the file:

```go
f, err := getFileReader(fileName)
if err != nil {
return err
}
defer func() {
if closer, ok := f.(io.Closer); ok {
_ = closer.Close()
}
}()
```

Note: The fix needs to handle both code paths - the initial open and the potential reopen after metadata reading failure.

Jira issue: CRDB-58850

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.