Bugzilla – Attachment 191435 Details for
Bug 41603
Plugin hook causing DB locks when cancelling holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41603: Move after_hold_action plugin hook outside transaction
8bc1f7d.patch (text/plain), 3.41 KB, created by
Tomás Cohen Arazi (tcohen)
on 2026-01-14 18:05:18 UTC
(
hide
)
Description:
Bug 41603: Move after_hold_action plugin hook outside transaction
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2026-01-14 18:05:18 UTC
Size:
3.41 KB
patch
obsolete
>From 8bc1f7d4a02b57b2cc85ee1099ddad571e44275c Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tom=C3=A1s=20Cohen=20Arazi?= <tomascohen@theke.io> >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 <tomascohen@theke.io> >--- > 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) >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41603
:
191435
|
191794
|
191797