From b81ed99571957e9fed43dab764f7bff209cc0dea Mon Sep 17 00:00:00 2001 From: Moritz Date: Mon, 15 Dec 2025 12:32:05 +0100 Subject: [PATCH] fix: prevent deadlocks by detecting existing transactions --- lib/membership/member.ex | 2 +- lib/mv/membership_fees/cycle_generator.ex | 69 +++++++++++++++-------- 2 files changed, 46 insertions(+), 25 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 5862a25..8e67570 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -763,7 +763,7 @@ defmodule Mv.Membership.Member do end # Regenerates cycles with new type/amount - # CycleGenerator uses its own transaction with advisory lock + # CycleGenerator detects if already in transaction and uses advisory lock accordingly defp regenerate_cycles(member_id) do case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member_id) do {:ok, _cycles} -> :ok diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 0727a62..4ed7ee7 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -188,36 +188,57 @@ defmodule Mv.MembershipFees.CycleGenerator do # Convert UUID to integer for advisory lock (use hash) lock_key = :erlang.phash2(member_id) - result = - Repo.transaction(fn -> - # Acquire advisory lock for this transaction - Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + # Check if we're already in a transaction (e.g., called from Ash action) + if Repo.in_transaction?() do + # Already in transaction: use advisory lock directly without starting new transaction + # This prevents nested transactions which can cause deadlocks + Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) - case fun.() do - {:ok, result, notifications} when is_list(notifications) -> - # Return result and notifications separately - {result, notifications} + case fun.() do + {:ok, result, notifications} when is_list(notifications) -> + # Notifications will be sent after the outer transaction commits + # Return in same format as non-transaction case for consistency + {:ok, result} - {:ok, result} -> - # Handle case where no notifications were returned (backward compatibility) - {result, []} + {:ok, result} -> + {:ok, result} - {:error, reason} -> - Repo.rollback(reason) - end - end) + {:error, reason} -> + {:error, reason} + end + else + # Not in transaction: start new transaction with advisory lock + result = + Repo.transaction(fn -> + # Acquire advisory lock for this transaction + Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) - # Extract result and notifications, send notifications after transaction - case result do - {:ok, {cycles, notifications}} -> - if Enum.any?(notifications) do - Ash.Notifier.notify(notifications) - end + case fun.() do + {:ok, result, notifications} when is_list(notifications) -> + # Return result and notifications separately + {result, notifications} - {:ok, cycles} + {:ok, result} -> + # Handle case where no notifications were returned (backward compatibility) + {result, []} - {:error, reason} -> - {:error, reason} + {:error, reason} -> + Repo.rollback(reason) + end + end) + + # Extract result and notifications, send notifications after transaction + case result do + {:ok, {cycles, notifications}} -> + if Enum.any?(notifications) do + Ash.Notifier.notify(notifications) + end + + {:ok, cycles} + + {:error, reason} -> + {:error, reason} + end end end