From 8bc1f7d4a02b57b2cc85ee1099ddad571e44275c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= Date: Tue, 13 Jan 2026 13:09:03 -0300 Subject: [PATCH] Bug 41603: Move after_hold_action plugin hook outside transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The after_hold_action plugin hook is currently called from within the hold cancellation transaction in Koha::Hold->cancel(). This creates a fragile architecture where any plugin performing database queries can cause lock timeouts and transaction failures. The issue manifests when plugins query tables with foreign key relationships to biblio records. Even lightweight queries on seemingly unrelated tables (like illrequests) can trigger lock conflicts due to foreign key constraints that require shared locks on referenced rows. Since the plugin hook executes while the transaction holds locks on reserves/biblio tables, any plugin database access creates potential for circular lock dependencies and timeouts. This patch moves the plugin hook call outside the transaction boundary, ensuring that: - Core hold operations complete without plugin interference - Plugins receive the same data but execute after transaction commit - Database lock conflicts are eliminated - Plugin failures cannot corrupt core hold cancellation logic The hook receives identical data through a stored reference to the cancelled hold, maintaining full backward compatibility while eliminating the transaction fragility. Test plan: 1. Install any plugin that implements after_hold_action hook with database queries (e.g., ILL plugins checking hold relationships) 2. Create a hold and attempt to cancel it 3. Without patch: Operation may timeout with "Lock wait timeout exceeded" 4. With patch: Operation completes immediately 5. Verify plugin hook still receives correct hold data Signed-off-by: Tomás Cohen Arazi --- Koha/Hold.pm | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 78eb4a5d38..a45df98d87 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -860,6 +860,9 @@ sub cancel { my $autofill_next = $params->{autofill} && $self->itemnumber && $self->found && $self->found eq 'W'; + # Store hold data for plugin hook (before transaction) + my $old_hold_data; + my $original = C4::Context->preference('HoldsLog') ? $self->unblessed : undef; $self->_result->result_source->schema->txn_do( @@ -916,13 +919,8 @@ sub cancel { my $old_me = $self->_move_to_old; - Koha::Plugins->call( - 'after_hold_action', - { - action => 'cancel', - payload => { hold => $old_me->get_from_storage } - } - ); + # Store data for plugin hook (to be called after transaction) + $old_hold_data = $old_me->get_from_storage; # anonymize if required $old_me->anonymize @@ -966,6 +964,15 @@ sub cancel { } ); + # Call plugin hook AFTER transaction completes + Koha::Plugins->call( + 'after_hold_action', + { + action => 'cancel', + payload => { hold => $old_hold_data } + } + ); + $self->cleanup_hold_group() unless $params->{skip_hold_group_cleanup}; if ($autofill_next) { -- 2.50.1 (Apple Git-155)