Bug 37548 - Race condition in CHECKIN notices
Summary: Race condition in CHECKIN notices
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 15854
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-01 16:31 UTC by Nick Clemens (kidclamp)
Modified: 2024-10-21 13:26 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-08-01 16:31:32 UTC
On bug 15854 we introduced a lock on the messages table to avoid generating too many checkin digest notices.

We saw on a live site a different issue recently.

A patron had a checkin notice with 17 items

process_message_queue ran and picked the contents of that notice for sending, but had not yet marked it sent

The patron returned 10 additional items
These were added to the notice in the DB

process_message_queue sent the digest with the contents from the initial fetch in the while loop

process message queue marked the notice as sent


Result:
Patron received notice of 17 items checked in 
DB notice had 27 and is marked sent
Comment 1 David Cook 2024-10-18 02:20:44 UTC
Yikes... that's not good.
Comment 2 Jonathan Druart 2024-10-21 13:26:28 UTC
C4/Letters.pm

@@ -1038,7 +1038,7 @@ sub SendQueuedMessages {
 
     $domain_limits = Koha::Notice::Util->load_domain_limits; # (re)initialize per run
     while( ( my $message_object = $unsent_messages->next ) && ( !$limit || $count_messages < $limit ) ) {
-        my $message = $message_object->unblessed;
+        my $message = $message_object->get_from_storage->unblessed;

What about refetching the letter at the start of the loop? It's cheap and could reduce this problem.