Database close fails without error with pending transaction on Windows

[zdandoh]

Hi, thank you very much for making this package available to the community.

I have discovered what seems to be a bug that occurs when closing a database with a pending transaction. I have observed this on Windows, but it seems like it might be a problem on other platforms as well. The bug can be reproduced with the following test:

func TestPendingTransactionFail(t *testing.T) {
	db, err := sql.Open("sqlite3", "test.db")
	if err != nil {
		t.Error(err)
	}

	_, err = db.Begin()
	if err != nil {
		t.Error(err)
	}

	err = db.Close()
	if err != nil {
		t.Error(err)
	}

	err = os.Remove("test.db")
	if err != nil {
		t.Error(err)
	}
}

The call to os.Remove fails with the error: remove test.db: The process cannot access the file because it is being used by another process. The "other process" in this case is just this process with an open file handle to the database. It seems that Close() should return an error if it fails to close the database. It currently returns nil even though the underlying fd isn't closed.

[rittneje]

I believe the bug (as in the underlying issue) is present on all platforms. However, you won't notice a problem on Unix right away because they treat files differently than Windows.

From experimenting, this is actually a bug in the Go standard library. It appears that Go doesn't actually call Close on the driver until the active transaction completes.

[itizir]

Sounds like a feature, not a bug:

Close is safe to call concurrently with other operations and will block until all other operations finish.
It may be useful to first cancel any used context and then call close directly after.

In other words you can't rely on Conn.Close to do the cancellation for you: you need to take care of it in your application before Close can go through.

EDIT: Oh wait, Close in your example doesn't block, and it only fails at Remove? Did I misunderstand something? To me it still looks like buggy application code, though... :/

[zdandoh]

That bit of the docs doesn't describe what is happening here. Blocking forever would be fairly reasonable/expected behavior. In this case Conn.Close immediately returns nil indicating no error, but silently leaks a file handle. Silently leaking a file handle is pretty unintuitive and seems like a bug.

[itizir]

Yeah, from reading the docs I expected the Close to block. 🤷

So there's likely a bug or two in there, but regardless I'd steer clear of using Conn.Close as transaction cancellation mechanism and would make sure to use contexts and/or explicit defer tx.Rollback() or something.