Skip to content

Commit fb10013

Browse files
committed
fix: close temp file before renaming it
After the ct_index temp file was written but before it was closed, we were renaming it to the final file. That causes issues since the handle to the temp file is still open and we're blocking ourselves with that. The solution is to extract the function to store the temp file and let the defer file.Close() close the file so that we can rename it afterwards.
1 parent 67838a9 commit fb10013

2 files changed

Lines changed: 35 additions & 18 deletions

File tree

internal/certificatetransparency/ct-watcher.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,10 @@ func (w *Watcher) Stop() {
251251
if config.AppConfig.General.Recovery.Enabled {
252252
// Store current CT Indexes before shutting down
253253
filePath := config.AppConfig.General.Recovery.CTIndexFile
254-
metrics.Metrics.SaveCertIndexes(filePath)
254+
err := metrics.Metrics.SaveCertIndexes(filePath)
255+
if err != nil {
256+
log.Printf("Failed to save CT index file: %v\n", err)
257+
}
255258
}
256259

257260
w.cancelFunc()
@@ -320,7 +323,11 @@ func (w *Watcher) CreateIndexFile(filePath string) error {
320323

321324
w.cancelFunc()
322325

323-
metrics.Metrics.SaveCertIndexes(filePath)
326+
saveErr := metrics.Metrics.SaveCertIndexes(filePath)
327+
if saveErr != nil {
328+
return saveErr
329+
}
330+
324331
log.Println("Index file saved to", filePath)
325332

326333
return nil

internal/metrics/logmetrics.go

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,14 @@ func (m *LogMetrics) SaveCertIndexesAtInterval(interval time.Duration, ctIndexFi
244244
defer ticker.Stop()
245245

246246
for range ticker.C {
247-
m.SaveCertIndexes(ctIndexFilePath)
247+
if err := m.SaveCertIndexes(ctIndexFilePath); err != nil {
248+
log.Printf("Error saving CT indexes at '%s': %s\n", ctIndexFilePath, err)
249+
}
248250
}
249251
}
250252

251253
// SaveCertIndexes saves the index of CTLogs to a file.
252-
func (m *LogMetrics) SaveCertIndexes(ctIndexFilePath string) {
254+
func (m *LogMetrics) SaveCertIndexes(ctIndexFilePath string) error {
253255
tempFilePath := ctIndexFilePath + ".tmp"
254256

255257
// Get the index data
@@ -260,39 +262,47 @@ func (m *LogMetrics) SaveCertIndexes(ctIndexFilePath string) {
260262
log.Panic(cerr)
261263
}
262264

265+
// Store index data in a temp file
266+
err := storeTempFile(tempFilePath, bytes)
267+
if err != nil {
268+
return err
269+
}
270+
271+
// Atomically move the temp file to the permanent file
272+
renameErr := os.Rename(tempFilePath, ctIndexFilePath)
273+
if renameErr != nil {
274+
return fmt.Errorf("could not rename CT index temp file: %w", renameErr)
275+
}
276+
277+
return nil
278+
}
279+
280+
// storeTempFile stores the passed data in the tempFilePath.
281+
func storeTempFile(tempFilePath string, bytes []byte) error {
263282
// Save data to a temporary file first
264283
file, openErr := os.OpenFile(tempFilePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
265284
if openErr != nil {
266-
log.Println("Could not save CT index to temporary file: ", openErr)
267-
return
285+
return fmt.Errorf("could not save CT index to temporary file: %w", openErr)
268286
}
269287
defer file.Close()
270288

271289
truncateErr := file.Truncate(0)
272290
if truncateErr != nil {
273-
log.Println("Error truncating CT index temp file: ", truncateErr)
274-
return
291+
return fmt.Errorf("could not save CT index to temporary file: %w", truncateErr)
275292
}
276293

277294
// TODO: check for short writes
278295
_, writeErr := file.Write(bytes)
279296
if writeErr != nil {
280-
log.Println("Error writing to CT index temp file: ", writeErr)
281-
return
297+
return fmt.Errorf("could not save CT index to temporary file: %w", writeErr)
282298
}
283299

284300
syncErr := file.Sync()
285301
if syncErr != nil {
286-
log.Println("Error syncing CT index temp file: ", syncErr)
287-
return
302+
return fmt.Errorf("could not save CT index to temporary file: %w", syncErr)
288303
}
289304

290-
// Atomically move the temp file to the permanent file
291-
renameErr := os.Rename(tempFilePath, ctIndexFilePath)
292-
if renameErr != nil {
293-
log.Println("Error renaming CT index temp file: ", renameErr)
294-
return
295-
}
305+
return nil
296306
}
297307

298308
// GetProcessedCerts returns the total number of processed certificates.

0 commit comments

Comments
 (0)