Bug 41603: Move after_hold_action plugin hook outside transaction
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 <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
parent
b3cea14d28
commit
8f7a1b861f
21
Koha/Hold.pm
21
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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue