Skip to content

fix: Prevent lost updates in concurrent Program/Module edits using select_for_update()#3894

Merged
arkid15r merged 2 commits intoOWASP:mainfrom
preeeetham:fix/3855-select-for-update
Feb 11, 2026
Merged

fix: Prevent lost updates in concurrent Program/Module edits using select_for_update()#3894
arkid15r merged 2 commits intoOWASP:mainfrom
preeeetham:fix/3855-select-for-update

Conversation

@preeeetham
Copy link
Contributor

@preeeetham preeeetham commented Feb 10, 2026

Summary

Fixes #3855 - Prevents lost updates when multiple admins edit the same program or module simultaneously by implementing pessimistic locking with select_for_update().

Changes

  • backend/apps/mentorship/api/internal/mutations/program.py

    • update_program(): Added select_for_update() to lock program row during updates
    • update_program_status(): Added select_for_update() to lock program row during status updates
  • backend/apps/mentorship/api/internal/mutations/module.py

    • update_module(): Added select_for_update() to lock module row during updates

How It Works

The select_for_update() method acquires a database row lock within the existing @transaction.atomic context:

  1. First admin to start editing acquires an exclusive lock on the row
  2. Concurrent edits are blocked and wait for the lock to be released
  3. Second admin's transaction proceeds with fresh data after first admin commits
  4. Changes are applied sequentially, preventing overwrites

Implementation Details

Per maintainer feedback (@arkid15r), chose select_for_update() as the least intrusive solution:

  • ✅ No schema changes required
  • ✅ No frontend changes required
  • ✅ Minimal code changes
  • ✅ Works with existing @transaction.atomic decorators

Test Plan

  • All backend tests pass (make test-backend)
  • No linter errors introduced
  • Manual testing: Verify concurrent edits are serialized correctly
  • Manual testing: Confirm no performance degradation under normal load

cc @arkid15r @kasya

OWASP#3855)

Apply pessimistic locking in update_program, update_program_status,
and update_module per maintainer feedback (select_for_update least intrusive).

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

Walkthrough

Row-level locking via select_for_update() is added to Program and Module mutation operations that retrieve records within atomic transactions. This ensures exclusive locks prevent concurrent modifications during read-modify-write sequences.

Changes

Cohort / File(s) Summary
Pessimistic Locking in Mutation Operations
backend/apps/mentorship/api/internal/mutations/module.py, backend/apps/mentorship/api/internal/mutations/program.py
Added select_for_update() chaining to record retrieval calls in mutation resolvers. Module update now locks row via select_for_update().get() in addition to existing select_related(). Program mutations apply same pattern to two update operations (update_program and update_program_status), replacing direct get() calls with select_for_update().get(key=...).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR implements a fix for issue #3855 by using pessimistic locking with select_for_update() to prevent lost updates, which directly addresses the core objective of preventing silent data overwrites in concurrent edits.
Out of Scope Changes check ✅ Passed All changes are scoped to the three mutation functions specified in the issue (update_program, update_program_status, update_module) with minimal, focused additions of select_for_update() calls.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the problem, solution, implementation details, and test status.
Title check ✅ Passed The title accurately describes the main change: preventing lost updates in concurrent edits using select_for_update(), which directly matches the code modifications and PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@preeeetham preeeetham changed the title fix(mentorship): Prevent lost updates in concurrent Program/Module edits using select_for_update() fix: Prevent lost updates in concurrent Program/Module edits using select_for_update() Feb 10, 2026
@preeeetham preeeetham marked this pull request as ready for review February 10, 2026 18:20
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@sonarqubecloud
Copy link

@arkid15r arkid15r enabled auto-merge February 11, 2026 03:09
@arkid15r arkid15r added this pull request to the merge queue Feb 11, 2026
Merged via the queue into OWASP:main with commit 92fa423 Feb 11, 2026
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Critical: Lost updates in concurrent Program/Module edits due to missing optimistic locking

2 participants

Comments