Skip to content

Pool transaction reset(conn) after checkin can clear another client’s aborted status and commit a failed transaction #349

Description

@lukaszsamson

DBConnection.transaction/3 for pooled connections can return the holder to the pool before reset(conn) runs. A late reset(conn) from one client can clear :aborted status set by the next client using the same holder.

This lets an outer transaction commit even though a nested transaction failed.

Reproduction:

Run this exs script against a local postgres. Note destructive operations

Mix.install([:postgrex, :db_connection])

main = self()

opts = [
  hostname: "localhost",
  username: "postgres",
  database: "postgres",
  pool_size: 1
]

{:ok, pool} = Postgrex.start_link(opts)

Postgrex.query!(pool, "DROP TABLE IF EXISTS d2_repro", [])
Postgrex.query!(pool, "CREATE TABLE d2_repro (id int)", [])

count = fn ->
  %{rows: [[n]]} = Postgrex.query!(pool, "SELECT count(*) FROM d2_repro", [])
  n
end

run_y = fn ->
  Postgrex.transaction(pool, fn conn ->
    Postgrex.query!(conn, "INSERT INTO d2_repro VALUES (1)", [])

    {:error, :inner} =
      Postgrex.transaction(conn, fn c -> Postgrex.rollback(c, :inner) end)

    send(main, :y_nested_failed)

    receive do
      :y_go -> :ok
    end

    :done
  end)
end

# --- control: no race -> INSERT must be rolled back ---
y0 = spawn_link(fn -> send(main, {:y_result, run_y.()}) end)

receive do
  :y_nested_failed -> :ok
end

send(y0, :y_go)

control =
  receive do
    {:y_result, res} -> res
  end

IO.puts("Control (no race): Y => #{inspect(control)}, rows in table: #{count.()}")

# --- race ---
x =
  spawn_link(fn ->
    {:ok, _} =
      Postgrex.transaction(pool, fn conn -> Postgrex.query!(conn, "SELECT 1", []) end,
        log: fn entry ->
          if entry.call == :commit do
            send(main, :x_in_window)

            receive do
              :x_go -> :ok
            end
          end
        end
      )

    send(main, :x_done)
  end)

receive do
  :x_in_window -> :ok
end

y = spawn_link(fn -> send(main, {:y_result, run_y.()}) end)

receive do
  :y_nested_failed -> :ok
end

send(x, :x_go)

receive do
  :x_done -> :ok
end

send(y, :y_go)

race =
  receive do
    {:y_result, res} -> res
  end

rows = count.()
IO.puts("Race:                      Y => #{inspect(race)}, rows in table: #{rows}")

if rows > 0 do
  IO.puts("\n*** BUG CONFIRMED ***")
end

Result:

Control (no race): Y => {:error, :rollback}, rows in table: 0
Race:                      Y => {:ok, :done}, rows in table: 1

Expected Behavior

Y’s outer transaction should return:

{:error, :rollback}

and no row committed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions