BUG 原因

开启预编译后，gorm (db *PreparedStmtDB) prepare 会拿预编译语句先到 stmts 查找是否存在预编译 (stmt) 对象，有则直接返回 stmt 对象，无则缓存预编译对象，再缓存预编译语句，(db *PreparedStmtDB) ExecContext 有报错则 delete(db.Stmts, query) 删除预编译对象，但未删除缓存预编译语句，会造成 gorm 发送预编译通过但 SQL 执行时错误(如不可重复) PreparedSQL 一直增长未释放数据

版本

  • go：1.22
  • gorm：1.25.8

复现

  1. gorm 配置
DB, _= gorm.Open(mysql.New(conf), &gorm.Config{
        PrepareStmt: true,
    })
  1. 调用创建方法
database.DB.Create(&m)
  1. 报错
 Duplicate entry

Gorm 预编译源码

type Stmt struct {
    *sql.Stmt
    Transaction bool
    prepared    chan struct{}
    prepareErr  error
}

type PreparedStmtDB struct {
    Stmts       map[string]*Stmt
    PreparedSQL []string
    Mux         *sync.RWMutex
    ConnPool
}

func NewPreparedStmtDB(connPool ConnPool) *PreparedStmtDB {
    return &PreparedStmtDB{
        ConnPool:    connPool,
        Stmts:       make(map[string]*Stmt),
        Mux:         &sync.RWMutex{},
        PreparedSQL: make([]string, 0, 100),
    }
}

func (db *PreparedStmtDB) GetDBConn() (*sql.DB, error) {
    if sqldb, ok := db.ConnPool.(*sql.DB); ok {
        return sqldb, nil
    }

    if dbConnector, ok := db.ConnPool.(GetDBConnector); ok && dbConnector != nil {
        return dbConnector.GetDBConn()
    }

    return nil, ErrInvalidDB
}

func (db *PreparedStmtDB) Close() {
    db.Mux.Lock()
    defer db.Mux.Unlock()

    for _, query := range db.PreparedSQL {
        if stmt, ok := db.Stmts[query]; ok {
            delete(db.Stmts, query)
            go stmt.Close()
        }
    }
}

func (sdb *PreparedStmtDB) Reset() {
    sdb.Mux.Lock()
    defer sdb.Mux.Unlock()

    for _, stmt := range sdb.Stmts {
        go stmt.Close()
    }
    sdb.PreparedSQL = make([]string, 0, 100)
    sdb.Stmts = make(map[string]*Stmt)
}

func (db *PreparedStmtDB) prepare(ctx context.Context, conn ConnPool, isTransaction bool, query string) (Stmt, error) {
    db.Mux.RLock()
    if stmt, ok := db.Stmts[query]; ok && (!stmt.Transaction || isTransaction) {
        db.Mux.RUnlock()
        // wait for other goroutines prepared
        <-stmt.prepared
        if stmt.prepareErr != nil {
            return Stmt{}, stmt.prepareErr
        }

        return *stmt, nil
    }
    db.Mux.RUnlock()

    db.Mux.Lock()
    // double check
    if stmt, ok := db.Stmts[query]; ok && (!stmt.Transaction || isTransaction) {
        db.Mux.Unlock()
        // wait for other goroutines prepared
        <-stmt.prepared
        if stmt.prepareErr != nil {
            return Stmt{}, stmt.prepareErr
        }

        return *stmt, nil
    }
    _, ok := db.Stmts[query]

    // cache preparing stmt first
    cacheStmt := Stmt{Transaction: isTransaction, prepared: make(chan struct{})}
    // 设置缓存
    db.Stmts[query] = &cacheStmt
    db.Mux.Unlock()
    _, ok = db.Stmts[query]
    // prepare completed
    defer close(cacheStmt.prepared)

    // Reason why cannot lock conn.PrepareContext
    // suppose the maxopen is 1, g1 is creating record and g2 is querying record.
    // 1. g1 begin tx, g1 is requeue because of waiting for the system call, now `db.ConnPool` db.numOpen == 1.
    // 2. g2 select lock `conn.PrepareContext(ctx, query)`, now db.numOpen == db.maxOpen , wait for release.
    // 3. g1 tx exec insert, wait for unlock `conn.PrepareContext(ctx, query)` to finish tx and release.
    stmt, err := conn.PrepareContext(ctx, query)
    if err != nil {
        cacheStmt.prepareErr = err
        db.Mux.Lock()
        delete(db.Stmts, query)
        db.Mux.Unlock()
        return Stmt{}, err
    }
    db.Mux.Lock()
    cacheStmt.Stmt = stmt
    // 设置缓存 SQL
    db.PreparedSQL = append(db.PreparedSQL, query)
    db.Mux.Unlock()

    return cacheStmt, nil
}

func (db *PreparedStmtDB) ExecContext(ctx context.Context, query string, args ...interface{}) (result sql.Result, err error) {
    stmt, err := db.prepare(ctx, db.ConnPool, false, query)
    if err == nil {
       result, err = stmt.ExecContext(ctx, args...)
       if err != nil {
          db.Mux.Lock()
          defer db.Mux.Unlock()
          go stmt.Close()
          // 删除缓存
          delete(db.Stmts, query)
       }
    }
    return result, err
}

Comment From: github-actions[bot]

The issue has been automatically marked as stale as it missing playground pull request link, which is important to help others understand your issue effectively and make sure the issue hasn't been fixed on latest master, checkout https://github.com/go-gorm/playground for details. it will be closed in 30 days if no further activity occurs. if you are asking question, please use the Question template, most likely your question already answered https://github.com/go-gorm/gorm/issues or described in the document https://gorm.io ✨ Search Before Asking ✨

Comment From: a631807682

PreparedSQL stores the key of Stmts, which gorm uses only when PreparedStmtDB Close. It is an exported field, is it used elsewhere? Can we remove it? @jinzhu

https://github.com/go-gorm/gorm/commit/c7667e9299134799da6f16e19eaf50cb8419736f

Comment From: ivila

PreparedSQL stores the key of Stmts, which gorm uses only when PreparedStmtDB Close. It is an exported field, is it used elsewhere? Can we remove it? @jinzhu

c7667e9

I think he uses this just trying to avoid deleting keys while iterating, in my opinion, removing the PreparedSQL field and using two iteration is better, just like the following:

// and PreparedStmtDB.PreparedSQL should be removed
func (db *PreparedStmtDB) Close() {
    db.mux.Lock()

    keys := make([]string, 0, len(db.Stmts))
    for key := range db.Stmts {
        keys = append(keys, key)
    }
    for _, key := range keys {
        if stmt, ok := db.Stmts[key]; ok {
            delete(db.Stmts, key)
            stmt.Close()
        }
    }

    db.mux.Unlock()
}

Should we? @jinzhu @a631807682

Comment From: a631807682

PreparedSQL stores the key of Stmts, which gorm uses only when PreparedStmtDB Close. It is an exported field, is it used elsewhere? Can we remove it? @jinzhu c7667e9

I think he uses this just trying to avoid deleting keys while iterating, in my opinion, removing the PreparedSQL field and using two iteration is better, just like the following:

``` // and PreparedStmtDB.PreparedSQL should be removed func (db *PreparedStmtDB) Close() { db.mux.Lock()

keys := make([]string, 0, len(db.Stmts)) for key := range db.Stmts { keys = append(keys, key) } for _, key := range keys { if stmt, ok := db.Stmts[key]; ok { delete(db.Stmts, key) stmt.Close() } }

db.mux.Unlock() } ```

Should we? @jinzhu @a631807682

Are you interested in creating a PR for it?

Comment From: ivila

Sure, just wait for hours, I will write a PR for it.

Comment From: ivila

@a631807682 during my coding, I see another potential bug in the Reset function: https://github.com/go-gorm/gorm/blob/4a50b36f638c6899089e6e3457425528ce693933/prepare_stmt.go#L59-L69 while we prepare a new statement, the new Stmt register itself to the db.Stmt map (with sql.Stmt* field value nil), and unlock the mutex: https://github.com/go-gorm/gorm/blob/4a50b36f638c6899089e6e3457425528ce693933/prepare_stmt.go#L96-L104

which result in a nil pointer error at Reset function call:

 func (sdb *PreparedStmtDB) Reset() { 
    sdb.Mux.Lock() 
    defer sdb.Mux.Unlock() 

    for _, stmt := range sdb.Stmts { 
                // stmt could be nil here
        go stmt.Close() 
    } 
    sdb.PreparedSQL = make([]string, 0, 100) 
    sdb.Stmts = make(map[string]*Stmt) 
 }

but as what I said, it is just a potential bug(people not likely call Reset and exec SQL concurrently).