Skip to content

Transaction not rolled back on early return in UpdateRecipeWithHistoryEntry #46

@windoze95

Description

@windoze95

Description

In internal/repository/recipe.go:238-256, a database transaction is started with tx := r.DB.Begin() on line 238. However, if the early-exit condition on lines 255-256 is hit (new active entry equals the current one), the function returns nil without calling either tx.Commit() or tx.Rollback(). This transaction is leaked — it holds a database connection until a server-side timeout fires.

Impact

  • Database connection pool exhaustion under load (many small updates hitting the no-op path)
  • Potential lock contention if the transaction holds any row locks before the early return

Suggested Fix

Add a deferred rollback at the top of the function, which is a no-op if the transaction was already committed:

tx := r.DB.Begin()
if tx.Error != nil {
    return tx.Error
}
defer func() {
    if r := recover(); r != nil {
        tx.Rollback()
    }
}()
// Or simply: defer tx.Rollback() — GORM's Rollback is a no-op after Commit

Then replace the early return nil on line 256 with:

tx.Rollback()
return nil

Note: This is distinct from #33 (which tracks the missing tx.Error check after Begin()) and #22 (nil pointer panic in the same function).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions