From c48f7aa23d31e314d2d510c7776d99c40fe99876 Mon Sep 17 00:00:00 2001 From: antv Date: Wed, 15 Jul 2026 22:35:50 +0700 Subject: [PATCH] Address Race Condition in sendThankcards using pessimistic locking (lockForUpdate) --- app/Services/User/UserService.php | 59 +++++++++++++++++++------------ 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/app/Services/User/UserService.php b/app/Services/User/UserService.php index d8c046a..4eec54e 100644 --- a/app/Services/User/UserService.php +++ b/app/Services/User/UserService.php @@ -47,36 +47,45 @@ class UserService implements UserServiceInterface $startOfMonth = Carbon::now()->startOfMonth(); $endOfMonth = Carbon::now()->endOfMonth(); - $cardsSentToThisUserThisMonth = Administration::where('msnv', $sender->msnv) - ->where('receiver', $receiverMsnv) - ->whereBetween('date', [$startOfMonth, $endOfMonth]) - ->sum('sent'); + DB::transaction(function () use ($sender, $receiverMsnv, $amount, $startOfMonth, $endOfMonth) { + // Lock sender record to prevent concurrent transaction modifications + $lockedSender = User::where('id', $sender->id)->lockForUpdate()->first(); - if ($cardsSentToThisUserThisMonth + $amount > Administration::MAX_SEND_CARD_PER_MONTH) { - throw new \App\Exceptions\ThankCardException( - __('messages.error.max_send_limit_template', [ - 'max' => Administration::MAX_SEND_CARD_PER_MONTH, - 'sent' => $cardsSentToThisUserThisMonth - ]) - ); - } + if ($lockedSender->flag_send == config('constants.FLAG_SEND_DISABLED')) { + throw new \App\Exceptions\ThankCardException(__('messages.error.no_send_permission')); + } - if ($sender->card < $amount) { - throw new \App\Exceptions\ThankCardException(__('messages.error.not_enough_cards')); - } + if ($lockedSender->card < $amount) { + throw new \App\Exceptions\ThankCardException(__('messages.error.not_enough_cards')); + } + + // Lock & check total sent cards to the receiver this month + $cardsSentToThisUserThisMonth = Administration::where('msnv', $lockedSender->msnv) + ->where('receiver', $receiverMsnv) + ->whereBetween('date', [$startOfMonth, $endOfMonth]) + ->lockForUpdate() + ->sum('sent'); + + if ($cardsSentToThisUserThisMonth + $amount > Administration::MAX_SEND_CARD_PER_MONTH) { + throw new \App\Exceptions\ThankCardException( + __('messages.error.max_send_limit_template', [ + 'max' => Administration::MAX_SEND_CARD_PER_MONTH, + 'sent' => $cardsSentToThisUserThisMonth + ]) + ); + } - DB::transaction(function () use ($sender, $receiverMsnv, $amount) { Administration::create([ 'msnv' => $receiverMsnv, 'received' => $amount, - 'sender' => $sender->msnv, + 'sender' => $lockedSender->msnv, 'sent' => 0, 'receiver' => null, 'date' => Carbon::today(), ]); Administration::create([ - 'msnv' => $sender->msnv, + 'msnv' => $lockedSender->msnv, 'received' => 0, 'sender' => null, 'sent' => $amount, @@ -84,12 +93,16 @@ class UserService implements UserServiceInterface 'date' => Carbon::today(), ]); - $sender->card -= $amount; - if ($sender->card <= 0) { - $sender->card = 0; - $sender->flag_send = config('constants.FLAG_SEND_DISABLED'); + $lockedSender->card -= $amount; + if ($lockedSender->card <= 0) { + $lockedSender->card = 0; + $lockedSender->flag_send = config('constants.FLAG_SEND_DISABLED'); } - $sender->save(); + $lockedSender->save(); + + // Synchronize variables back to the original model instance + $sender->card = $lockedSender->card; + $sender->flag_send = $lockedSender->flag_send; }); }