How to get your database access code deadlocked in Golang

Unlike C++ or Rust, Golang is supposed to be a programmer friendly language. Most of the time it does a great job in increasing developer productivity by being a relatively easy programming language and trying not to surprise its users with hidden behaviours. Unfortunately, every now and then there are cases when a simple code can yield unexpected results. I will present a simple way to deadlock a database access code that uses database/sql package from Golang standard library.

I run into the problem described below while working on Golang driver for Turso database. It manifested itself in much more complex scenario but the minimal example that reproduces the issue is surprisingly small.

Following function creates a users table and populates it with some data.

func createUsersTable(db *sql.DB) error {
 _, err := db.Exec("CREATE TABLE users (email TEXT)")
 if err != nil {
  return err
 }
 for i := 0; i < 10; i++ {
  email := gofakeit.Email()
  _, err = db.Exec("INSERT INTO users (email) VALUES (?)", email)
  if err != nil {
   return err
  }
 }
 return nil
}

Now let's consider a function that processes each user in the users table.

func processUsers(ctx context.Context, db *sql.DB) error {
 conn, err := db.Conn(ctx)
 if err != nil {
  return err
 }
 defer conn.Close()
 rows, err := conn.QueryContext(ctx, "SELECT email FROM users")
 if err != nil {
  return err
 }
 for rows.Next() {
  var email string
  if err := rows.Scan(&email); err != nil {
   return err
  }
  if err := processUser(email); err != nil {
   return err
  }
 }
 return nil
}

We can ignore the details of processUser function. All we have to know about it is that it does some processing using the email of the user and that this processing can fail and then an error is returned from the function.

Now, does this function look correct? Would you find out while reviewing the code that it can deadlock? I encourage you to take a few minutes to try to find what's wrong with this code by yourself before you keep reading.

This code will work fine in a happy path when no errors occur but otherwise it can deadlock just by itself.

This function deadlocks when error is returned from within the for loop preventing the rows.Next()from being called until it returns false.

conn is of type sql.Conn which has Reader-Writer lock. When conn.Close()is executed it wants to obtain a writer lock. At the same time, it turns out that rowskeeps a reader lock until it's fully read or rows.Close() is called explicitly.

On a happy path, the for loop iterates until rows is exhausted and closed implicitly which leads to its reader lock being released. Then a deferred call to conn.Close() can safely acquire a writer lock.

The deadlock appears when the for loop finishes early because of an error being returned. That leaves the rows with some more data to be read so rowskeeps its reader lock. In such a case, a deferred call to conn.Close()can't obtain a writer lock and has to wait on the lock leading to a deadlock.

The fix is very simple. All we have to do is to make sure that rows are always closed.

func processUsers(ctx context.Context, db *sql.DB) error {
 conn, err := db.Conn(ctx)
 if err != nil {
  return err
 }
 defer conn.Close()
 rows, err := conn.QueryContext(ctx, "SELECT email FROM users")
 if err != nil {
  return err
 }
 defer rows.Close() // <-- THIS WAS ADDED
 for rows.Next() {
  var email string
  if err := rows.Scan(&email); err != nil {
   return err
  }
  if err := processUser(email); err != nil {
   return err
  }
 }
 return nil
}

One could say that this code uses db.Connso it's an advanced usage of the database/sql package so the user should know what they are doing. The problem is that the deadlock appears even with simpler more basic usage like the following processUsers2 function. It's just easier to demonstrate it by using sql.Conn directly.

func processUsers2(ctx context.Context, db *sql.DB) error {
 rows, err := db.QueryContext(ctx, "SELECT email FROM users")
 if err != nil {
  return err
 }
 for rows.Next() {
  var email string
  if err := rows.Scan(&email); err != nil {
   return err
  }
  if err := processUser(email); err != nil {
   return err
  }
 }
 return nil
}

This call to db.QueryContext uses sql.Conn underneath so not closing the rows object will leave the connection in a bad state. That will manifest next time some call tries to take this connection from the connection pool and use it. That would end up with deadlock again. It can be easily simulated by reducing the size of the connection pool to 1 with the following code and calling processUsers2 twice.

db.SetMaxOpenConns(1)

#Conclusions

The code example in this article leads to a deadlock because it's incorrect. It forgets to close the result of the QueryContext. Nevertheless, failing to close a query result leading to a deadlock is a bit unexpected result. What makes it even worse is that the code does not deadlock on a happy path when no errors occur. That's usually the most tested path while the error paths generally don't receive as much attention while testing. This makes the described issue especially problematic so be careful and always remember to close the result of database query when you're using database/sql package.

#EDIT

Jonathan Hall pointed me to some existing Golang issues that are related to the problem described in this article. Some as old as 2019.

#EDIT 2

A follow up article describing some more unexpected behaviours you can see when leaking rows -> /blog/something-you-probably-want-to-know-about-if-youre-using-sqlite-in-golang-72547ad625f1