Skip to content

Commit 693d722

Browse files
committed
fix(store/file): close ODS file descriptors in ValidateODSSize and OpenODS
ValidateODSSize opened an ODS via OpenODS but never closed it on any path, leaking one file descriptor per call during store startup validation. OpenODS itself also leaked the file when readHeader failed (e.g. on a truncated or corrupt file), since the descriptor was never returned to the caller. Close the ODS in ValidateODSSize via defer, and close the file in OpenODS when header parsing fails. Adds regression tests that count open descriptors and fail on the pre-fix leak.
1 parent b4738cb commit 693d722

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

store/file/ods.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ func ValidateODSSize(path string, eds *rsmt2d.ExtendedDataSquare) error {
142142
if err != nil {
143143
return fmt.Errorf("opening file: %w", err)
144144
}
145+
defer ods.Close()
145146

146147
shares, err := filledSharesAmount(eds)
147148
if err != nil {
@@ -173,6 +174,7 @@ func OpenODS(path string) (*ODS, error) {
173174

174175
h, err := readHeader(f)
175176
if err != nil {
177+
_ = f.Close()
176178
return nil, err
177179
}
178180

store/file/ods_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"io"
77
"os"
8+
"runtime"
89
"strconv"
910
"testing"
1011
"time"
@@ -153,6 +154,59 @@ func TestValidateODSSize(t *testing.T) {
153154
}
154155
}
155156

157+
// openFDCount returns the number of file descriptors currently open by the
158+
// process. It relies on Linux's /proc and skips the test elsewhere.
159+
func openFDCount(t *testing.T) int {
160+
t.Helper()
161+
if runtime.GOOS != "linux" {
162+
t.Skip("file descriptor counting via /proc is only available on Linux")
163+
}
164+
entries, err := os.ReadDir("/proc/self/fd")
165+
require.NoError(t, err)
166+
return len(entries)
167+
}
168+
169+
// TestValidateODSSize_NoFDLeak ensures ValidateODSSize does not leak the file
170+
// descriptor opened by OpenODS. It opened the file but never closed it on any
171+
// path, leaking one fd per call during store startup validation.
172+
func TestValidateODSSize_NoFDLeak(t *testing.T) {
173+
eds := edstest.RandEDS(t, 8)
174+
roots, err := share.NewAxisRoots(eds)
175+
require.NoError(t, err)
176+
path := t.TempDir() + "ods-fd-leak"
177+
require.NoError(t, CreateODS(path, roots, eds))
178+
179+
// warm up one call so any one-off fds (not per-call leaks) are already open.
180+
require.NoError(t, ValidateODSSize(path, eds))
181+
182+
before := openFDCount(t)
183+
for range 50 {
184+
require.NoError(t, ValidateODSSize(path, eds))
185+
}
186+
after := openFDCount(t)
187+
require.LessOrEqual(t, after-before, 1, "ValidateODSSize leaked file descriptors")
188+
}
189+
190+
// TestOpenODS_NoFDLeakOnHeaderError ensures OpenODS closes the underlying file
191+
// when header parsing fails, instead of leaking the descriptor.
192+
func TestOpenODS_NoFDLeakOnHeaderError(t *testing.T) {
193+
// too short to contain a valid header, so readHeader fails.
194+
path := t.TempDir() + "corrupt-ods"
195+
require.NoError(t, os.WriteFile(path, []byte{0x00, 0x01, 0x02}, 0o600))
196+
197+
// warm up so the first-call allocations don't skew the count.
198+
_, err := OpenODS(path)
199+
require.Error(t, err)
200+
201+
before := openFDCount(t)
202+
for range 50 {
203+
_, err := OpenODS(path)
204+
require.Error(t, err)
205+
}
206+
after := openFDCount(t)
207+
require.LessOrEqual(t, after-before, 1, "OpenODS leaked a file descriptor on header error")
208+
}
209+
156210
// BenchmarkAxisFromODSFile/Size:32/ProofType:row/squareHalf:0-16 382011 3104 ns/op
157211
// BenchmarkAxisFromODSFile/Size:32/ProofType:row/squareHalf:1-16 9320 122408 ns/op
158212
// BenchmarkAxisFromODSFile/Size:32/ProofType:col/squareHalf:0-16 4408911 266.5 ns/op

0 commit comments

Comments
 (0)