Bug 41603

Summary: Plugin hook causing DB locks when cancelling holds
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Plugin architectureAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Needs documenting --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: critical    
Priority: P5 - low CC: Chip.Halvorsen, dbarden, kkrueger, Laura.escamilla, lisette, lthorrat, lucas, nick, tomascohen
Version: MainKeywords: release-notes-needed
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38384
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
26.05.00,25.11.02
Circulation function:
Attachments: Bug 41603: Move after_hold_action plugin hook outside transaction
Bug 41603: Move after_hold_action plugin hook outside transaction
Bug 41603: Move after_hold_action plugin hook outside transaction

Description Tomás Cohen Arazi (tcohen) 2026-01-13 15:42:52 UTC
The Rapido ILL plugin [1] has a simple `after_hold_action` hook that checks if the current hold is linked to an ILL request [2], and in the positive case, it inserts a row on a `task_queue` table for the action to be processed asynchronously by a separate daemon/worker.

Even though the plugin itself is not doing any heavy action on the DB, this causes DB locks that are easily reproducible. This seems to be a core Koha issue as it makes the holds cancelling process (and probably others) rather fragile.

To reproduce:
1. Clone Rapido ILL:
   $ mkdir -p ~/git/koha-plugins && cd ~/git/koha-plugins
   $ git clone https://github.com/bywatersolutions/koha-plugin-rapido-ill.git rapido-ill
2. Start a KTD instance:
   $ ktd --proxy --name rapido --single-plugin ~/git/koha-plugins/rapido-ill up -d
   $ ktd --name rapido --wait-ready 200
3. Install the plugin config
   $ ktd --name rapido --shell
  k$ cd /kohadevbox/plugins/rapido-ill/scripts
  k$ perl bootstrap_rapido_testing.pl
  k$ restart_all
4. Create a hold for a known patron
5. Run this:
  k$ perl -e 'use Carp::Always; use Koha::Holds; my $hold = Koha::Holds->find(1);$hold->cancel({cancellation_reason => "OVER_365"})'
=> FAIL: It takes a while! It eventually fails with a lock wait timeout!:

```
C4::Reserves::_FixPriority(): DBI Exception: DBD::mysql::st execute failed: Lock wait timeout exceeded; try restarting transaction at /kohadevbox/koha/Koha/Hold.pm line 933
```

=> FACTS: There's a hold, being cancelled. There is no ILL request on the system. The only thing that is happening is that the plugin hook is being called, and is doing this query:

```
MariaDB [koha_cuyahoga]> SELECT me.*
    FROM illrequests me
    JOIN illrequestattributes
    ON illrequestattributes.illrequest_id = me.illrequest_id
    WHERE illrequestattributes.type = 'hold_id'
    AND illrequestattributes.value = '1'
    AND me.backend = 'RapidoILL';
Empty set (0.001 sec)
```

Moving the plugin hook outside the transaction fixes it.

[1] https://github.com/bywatersolutions/koha-plugin-rapido-ill
[2] Linked means checking if it has an extended attribute with the hold_id value. No FK checks.
Comment 1 Lucas Gass (lukeg) 2026-01-14 15:17:33 UTC

*** This bug has been marked as a duplicate of bug 40220 ***
Comment 2 Tomás Cohen Arazi (tcohen) 2026-01-14 15:18:22 UTC
(In reply to Lucas Gass (lukeg) from comment #1)
> 
> *** This bug has been marked as a duplicate of bug 40220 ***
Comment 3 Tomás Cohen Arazi (tcohen) 2026-01-14 18:05:18 UTC
Created attachment 191435 [details] [review]
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>
Comment 4 Nick Clemens (kidclamp) 2026-01-21 16:06:56 UTC
Created attachment 191794 [details] [review]
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>
Comment 5 Nick Clemens (kidclamp) 2026-01-21 16:12:17 UTC
https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/releases
I tested with the KitchenSink plugin, adding an after_hold_action routine, and was able to cancel a hold placed in the staff interface both before and after from the interface and using:
echo "UPDATE reserves SET reservedate='2025-01-01'" | sudo koha-mysql kohadev; perl misc/cronjobs/holds/cancel_unfilled_holds.pl -d 35 -v -c


https://github.com/bywatersolutions/koha-plugin-rapido-ill/releases
When I added the RapidoPLugin, the interface worked, but the command line failed

After the patch all methods succeeded. 

It also makes sense to only send the hold to the plugin after the transaction has succeeded, and not during
Comment 6 Martin Renvoize (ashimema) 2026-01-21 16:26:33 UTC
Created attachment 191797 [details] [review]
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>
Comment 7 Lucas Gass (lukeg) 2026-01-22 16:28:09 UTC
Nice work everyone!

Pushed to main for 26.05
Comment 8 Lucas Gass (lukeg) 2026-01-22 16:32:04 UTC
please add release notes
Comment 9 Jacob O'Mara 2026-02-16 15:45:26 UTC
Thanks all, pushed to 25.11.x
Comment 10 Laura Escamilla 2026-02-27 16:23:09 UTC
I am not backporting this as it does not apply cleanly to 25.05.x.