go-sqlite3 icon indicating copy to clipboard operation
go-sqlite3 copied to clipboard

Potential memory leak on Sql.Db.Close()

Open Harmos274 opened this issue 10 months ago • 6 comments

sqlite3 version: github.com/mattn/go-sqlite3 v1.14.22

Hello, i would like to know if i'm doing something wrong here... It seems that sql.Db.Close() does not release allocated memory when called.

In fact, i suspect it to cause a memory leak.

I have a project that is a net/http api that exposes a GET /instances/{instance}/jobs/{jobID}/status route.

  • instance is a hash string that i get the name of an sqlite database file from.
  • jobID is the ID of a job in the given database.

My service will take the database {instance}.sqlite, opens it and query the status of the job {jobID}, Afterward it will close the given database.

flowchart LR;
    A[Http GET]-->B[net/http API];
    B-->C[Sql.Open instance.sqlite];
    C-->D[Query Job status];
    D-->E[Sql.DB.Close instance.sqlite];
    E-->F[Http response];

Here's a code sample that shows how i open and close both of my databases.

func onDatabaseContext[T any](ctx context.Context, dbManager SetupDB, instance string, onDBFunc func(ctx context.Context, writeDB, readDB *sql.DB) (T, error)) (written T, err error) {
	var writeDB, readDB *sql.DB

        // Compute sqlite db path from instance number
	path := dbManager.dbPath(instance)

	// Write in sqlite is mono threaded, therefore we use a multithreaded read db and mono threaded write db
	writeDB, err = sql.Open("sqlite3", fmt.Sprintf("file:%s?mode=rw&_journal=WAL&_timeout=5000&_txlock=immediate", path))
	if err != nil {
		return
	}

	readDB, err = sql.Open("sqlite3", fmt.Sprintf("file:%s?mode=ro&_timeout=5000&parseTime=true", path))
	if err != nil {
		return
	}

	err = initDB(ctx, writeDB)

	if err != nil {
		return
	}

	res, err := onDBFunc(ctx, writeDB, readDB)

	writeDB.Close()
	readDB.Close()

	return res, err
}

The problem is that when i close my Sql.Db handles, the allocated memory is not released, even after some time. I actually work on a lot of different sql databases which causes my service to crash from OOM error eventually.

I'm sure my pattern is not efficient, it is a proof of concept. But i would expect resources to be released when i close my handles, or at least be garbage collected after some time, which does not happen.

Could someone explain to me what i am doing wrong please ?

Thank you for your time.

Harmos274 avatar Feb 20 '25 14:02 Harmos274

I'm new to Go, but isn't it better to put writeDB.Close() and readDB.close(0 in defer for this scenario? Correct me if I'm wrong, but for each error checking, if the error is not nil, the function will return an empty value. Other codes below whichever failed error checking will not executed after that. So if one of the error checking fails, the function will just finish without even touching the writeDB.close() and readDB.close(). By using defer it make sure that whatever happens, the database will be closed before the function is close.

yafethtb avatar Apr 23 '25 15:04 yafethtb

absolutely! tbf it was the original version of my code, i wanted to be sure that .Close() is called after onDbFunc(), to exclude the hypothesis of "defer is not executed after my closure", the problem i'm talking about in that issue is happening when none of the previous calls return an error

Harmos274 avatar Apr 23 '25 16:04 Harmos274

Well, firstly and most obviously you are eating errors and returning early. This isn't how you should handle errors in Go. your Close calls should be done in deferrals immediately after checking whether Open returned an error or not.

As far as GC/memory leak goes, does your onDbFunc make use of goroutines perhaps? i would get rid of the callback functionality and run some tests just to confirm. Maybe something there holding onto a reference.

jmcdonagh avatar Apr 23 '25 17:04 jmcdonagh

Issue #1328 also faces a memory leak problem. It's probably similar to what you've faced.

yafethtb avatar Apr 24 '25 07:04 yafethtb

@jmcdonagh as i said earlier i was facing the same issue when i was using defer i'm not sure my error would come from this part of the code (but yea in this case errors cause a memory leak, i wasn't facing any error tho, everything was fine during the execution)

no goroutines at all in the callback, just classic sql execution mainly select and return the scanned rows (i close my rows)

Harmos274 avatar Apr 24 '25 07:04 Harmos274

@yafethtb it could be the case indeed :/

Harmos274 avatar Apr 24 '25 07:04 Harmos274