Skip to content

Conversation

@Aryanpagaria
Copy link

Fixes #4830

Summary

When is_restricted_to_select_one_submission=True on a challenge phase, two or more concurrent requests can toggle different submissions as public at the same time. This happens because the visibility check uses a non-atomic .count() query, allowing both requests to observe stale state and proceed, resulting in multiple public submissions for the same team and phase.

This PR introduces an atomic database-level lock using transaction.atomic() and select_for_update() to guarantee exclusive access to the existing public submissions when toggling visibility.

This ensures the business rule is always enforced:

  • Only one submission can be public at any time
  • The currently public submission is set to private before a new one is made public
  • Works safely under concurrency

Root Cause

The existing logic:
is vulnerable to race conditions:

  1. Request A checks count() == 1 → True
  2. Request B checks count() == 1 → True (no change committed yet)
  3. Both requests proceed and make their submissions public

Because there is no row-level lock, both transactions succeed.

Fix

  • Wrap the public submission mutation logic inside transaction.atomic()
  • Use .select_for_update() to lock existing public submission rows until the update completes
  • Ensure correct ordering of operations before creating a new public submission
  • Remove serializer-based update to avoid nested save behavior inside the critical section

Technical Details

  • Uses PostgreSQL row-level locking
  • No behavioral change for phases without the restriction flag
  • Minimal diff, fully backward-compatible

Notes

This PR does not modify the public/private rules beyond fixing the race condition.
The logic path for non-public submissions is unaffected.

Testing

  • Verified basic functionality manually
  • Confirmed expected behavior under concurrent requests using the reproduction script provided in the issue description
  • Existing test suite passes, and no regressions observed

Let me know if you would like me to add explicit unit tests for the concurrency scenario.

Fixes Cloud-CV#4830

Summary
When is_restricted_to_select_one_submission=True, concurrent requests that toggle submission visibility can result in multiple submissions being marked as public. This is caused by a non-atomic count() check that allows two parallel requests to observe stale state and both proceed, leading to more than one public submission.

Root Cause
The existing logic performs:

submissions_alread
@Aryanpagaria
Copy link
Author

Hi,
The Travis CI failure appears to be unrelated to the backend changes in this PR.
From the logs, the build fails in the CI environment itself (before any Django/backend tests or code-quality checks are run), and I don’t see any errors tied to the submission visibility logic or the new transaction.atomic() block.

Whenever convenient, could someone please retrigger the CI job or review the patch manually?

/cc @taranjeet @deshraj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Race Condition in Public Submission Toggling Allows Multiple Public Submissions

1 participant