From 88cd9df1610cedc94dc5671bc5aa2e1c121ce983 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 Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- Koha/Hold.pm | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index 28041bdfaa5..d030282997b 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.52.0