A Django concurrency story about read-then-mutate, silent no-ops, and why one fix is never enough.
Section 1: The Bug That Didn't Announce Itself
After I fixed the add_member race condition, I did what most developers do after a win: I closed the ticket and moved on.
Then a small voice asked a very inconvenient question:
If
add_memberhad a race condition, what aboutchange_member_role?
That question turned into an audit. And the audit turned into two more bugs.
Here's the thing about race conditions: they don't travel alone. When you find one check-then-act pattern, you're not looking at an isolated mistake — you're looking at a category. The same developer (me) wrote similar code in similar places, and the same class of bug is hiding in all of them.
I pulled up every mutation in my WorkspaceService:
WorkspaceService
├── create → atomic ✅
├── add_member → fixed in previous article ✅
├── change_member_role → ? ← here
├── remove_member → ? ← and here
├── transfer_ownership → select_for_update ✅
└── delete → atomic ✅
Two methods stood out immediately.
@staticmethod
def change_member_role(*, workspace, user, role, actor):
with transaction.atomic():
membership = Membership.objects.get(workspace=workspace, user=user) # ← no lock!
# ...
membership.role = role
membership.save()
NotificationService.create(...)
return membership
@staticmethod
def remove_member(*, workspace, user, actor):
with transaction.atomic():
membership = Membership.objects.get(workspace=workspace, user=user) # ← no lock!
# ...
membership.delete()
return True
No lock. No select_for_update. Just a plain get(), a mutation, and a save.
I had a suspicion. But suspicion isn't proof. So I wrote tests.
Section 2: The Contracts That Came Before the Tests
Before I could write a test, I had to decide what correct behavior even meant. This is where the second bug started teaching me things the first one hadn't.
There's a subtle trap in concurrency testing: if you don't define the contract precisely, you'll end up asserting on things that are inherently non-deterministic. So I wrote three contracts — one strict, one loose, one clever.
Contract 1: Same-Role Concurrent Change
B = MEMBER
Thread 1 (admin_a): B → ADMIN
Thread 2 (admin_c): B → ADMIN
Expected:
✅ Both operations succeed (the second is a no-op)
✅ Exactly one membership
✅ Final role = ADMIN
✅ Exactly one WORKSPACE_ROLE_CHANGED notification
✅ No IntegrityError
Key insight: this contract is different from add_member's contract.
For add_member, a duplicate is an error (400). For change_member_role, the same role is a no-op (200).
Why? Because the semantics are different:
add_member(B) again
→ B really doesn't want to join twice
→ conflict → 400
change_member_role(B, ADMIN) again
→ B is already ADMIN
→ desired state achieved
→ no-op → 200
This distinction is not cosmetic. It changes what you assert on, and — as we'll see — it changes how you catch the bug.
Contract 2: Different-Role Concurrent Change
Thread 1: B (MEMBER) → ADMIN
Thread 2: B (MEMBER) → MEMBER
Expected:
⚠️ Winner is non-deterministic
✅ Exactly one membership
✅ Final role ∈ {MEMBER, ADMIN}
✅ No IntegrityError
Here I couldn't assert on the final role — transaction ordering isn't deterministic. This is an exploration test, not a strict contract. It exists to catch invariant violations, not to enforce a specific outcome.
Contract 3: Role Change + Remove
Thread 1: B → ADMIN
Thread 2: remove(B)
Expected:
✅ Final count = 0 (in either ordering)
✅ No IntegrityError
Why count = 0?
- If role change commits first → B is ADMIN → owner can still remove an admin → count = 0
- If remove commits first → role change hits
DoesNotExist→ count = 0
Both orderings converge. That's the mark of a good concurrency test — it holds regardless of which thread wins.
Section 3: The First RED — A Duplicate Notification
I wrote the same-thread setup I'd used before: two real threads, a threading.Barrier(2), real transactions, and connection.close() in a finally.
@pytest.mark.django_db(transaction=True)
def test_concurrent_same_role_change(self):
workspace, owner, admin_a, admin_c, target = self._setup()
def change_a():
return WorkspaceService.change_member_role(
workspace=workspace, user=target,
role=MembershipRole.ADMIN, actor=admin_a,
)
def change_c():
return WorkspaceService.change_member_role(
workspace=workspace, user=target,
role=MembershipRole.ADMIN, actor=admin_c,
)
successes, errors = self._run_concurrent(change_a, change_c)
assert len(successes) == 2
assert len(errors) == 0
memberships = Membership.objects.filter(workspace=workspace, user=target)
assert memberships.count() == 1
assert memberships.first().role == MembershipRole.ADMIN
role_notifs = Notification.objects.filter(
type=Notification.Type.WORKSPACE_ROLE_CHANGED,
)
assert role_notifs.count() == 1
It failed. But not on the assertion I expected.
FAILED
AssertionError: Expected exactly 1 role-change notification, got 2
The membership count was correct. The final role was correct. The database hadn't corrupted anything.
What had gone wrong was invisible to the database.
Here's the timeline:
Thread A Thread B
│ │
├─ get() → MEMBER │
│ (no lock) ├─ get() → MEMBER
│ │ (no lock)
├─ role = ADMIN │
├─ save() ├─ role = ADMIN
├─ NotificationService ├─ save()
│ .create() │
│ → notif #1 ├─ NotificationService
│ │ .create()
├─ commit │ → notif #2
│ │
│ ├─ commit
└── └──
Both threads read MEMBER before either one committed. Both saw a real transition. Both wrote a notification.
From the database's perspective, everything is fine. From the user's perspective, they just got two notifications for one role change.
This is the same class of bug as check-then-create, but wearing a different mask:
# Anti-pattern #1 (previous article):
check() → create()
# Anti-pattern #2 (this article):
read() → mutate() → save()
Different shape. Same underlying flaw: a window between decision and action that isn't protected against concurrent access.
Section 4: The Fix — One Line
The right pattern already existed in the codebase. transfer_ownership was using it:
actor_membership = Membership.objects.select_for_update().get(...)
new_owner_membership = Membership.objects.select_for_update().get(...)
So the fix was a single line per method:
# Before
membership = Membership.objects.get(workspace=workspace, user=user)
# After
membership = (
Membership.objects
.select_for_update()
.get(workspace=workspace, user=user)
)
Why does this work?
select_for_update in PostgreSQL takes a row-level exclusive lock. While Thread A holds it, Thread B waits. When Thread A commits and releases the lock, PostgreSQL hands Thread B the new version of the row — not a stale snapshot.
So:
Thread A Thread B
│ │
├─ SELECT FOR UPDATE │
│ → lock acquired │
│ → MEMBER read ├─ SELECT FOR UPDATE
│ │ → waiting for lock
├─ role = ADMIN │
├─ save() │
├─ NotificationService │
│ .create() → notif #1 │
├─ commit │
│ → lock released │
│ ├─ lock acquired
│ ├─ re-read → ADMIN
│ ├─ role == ADMIN
│ │ → return (no-op)
│ ├─ commit
└── └──
The check if membership.role == role: return membership was already in the code — it had just never been able to do its job because both threads were reading stale data.
One line. One fix. One bug gone.
Section 5: The Second Bug — A Silent No-Op
The remove_member method had the same problem. But it had an additional twist that I hadn't anticipated.
Consider this scenario:
Thread A (change): read(B) → B=MEMBER
Thread B (remove): read(B) → delete(B) → commit
Thread A: B.role = ADMIN → save()
What does save() do when the row no longer exists?
The answer surprised me: nothing. Django sends an UPDATE ... WHERE id = ..., and the database matches zero rows. No error is raised. Silent no-op.
But the code continues:
membership.role = role
membership.save() # silent no-op
NotificationService.create(...) # ❌ notification created
Result: a WORKSPACE_ROLE_CHANGED notification for a role change that never actually happened.
This is worse than the duplicate-notification bug. In the duplicate case, at least something real happened. Here, the entire event is a ghost — a notification with no corresponding state change.
The fix was the same: add select_for_update. Now Thread A waits for Thread B to commit, and then hits DoesNotExist — a loud, visible failure instead of a silent one.
with transaction.atomic():
membership = (
Membership.objects
.select_for_update()
.get(workspace=workspace, user=user)
)
Two methods. Two bugs. Two lines.
Section 6: The Third Test — A Test Almost Broke
The contract for role-change + remove looked simple:
Thread 1: B → ADMIN
Thread 2: remove(B)
Count should be 0 in either ordering.
But it almost wasn't order-independent. Here's what I got wrong first:
I initially wrote the test using an admin to perform the removal.
Ordering 1:
A: B → ADMIN
C: remove(B) as ADMIN
→ "Admin cannot remove another admin"
→ count = 1 ❌
Ordering 2:
C: remove(B) as ADMIN
→ ok
A: B → DoesNotExist
→ count = 0 ✅
That's order-dependent — the test would pass or fail depending on which thread won. Useless.
The fix was to change the actor to the owner:
Ordering 1:
A: B → ADMIN
Owner: remove(B)
→ owner can remove admin → count = 0 ✅
Ordering 2:
Owner: remove(B)
→ ok
A: B → DoesNotExist → count = 0 ✅
Now the test is order-independent. It holds no matter which thread commits first.
The lesson: in concurrency tests, choose roles that make the test order-independent. If you can't, you're not testing an invariant — you're testing a coin flip.
Section 7: One More Consideration — Deadlock Potential
Once I decided to add select_for_update, a new question surfaced: can this create deadlocks?
Deadlocks happen when two transactions try to lock the same set of rows in different orders:
Thread A: lock(X) → lock(Y) → ...
Thread B: lock(Y) → lock(X) → ... ← deadlock
So I checked my remove_member implementation carefully:
# target: locked
membership = Membership.objects.select_for_update().get(...)
# actor: NOT locked (plain read)
actor_membership = Membership.objects.get(...)
Only one row gets locked — the target membership. The actor's membership is read without a lock.
That means no thread can ever hold two locks in a conflicting order. Deadlock is impossible by construction.
Had I locked the actor as well — which is a natural thing to want to do — I would have opened the door to deadlocks in multi-step operations.
The principle: lock the minimum set of rows required, and lock them in a consistent order. More locks ≠ safer. More locks = more chances to deadlock.
Section 8: Five Lessons From the Second Bug
This second round of bug-hunting taught me things the first one couldn't.
1. check-then-act isn't the only race-prone pattern
The first article was about check-then-create. This one is about read-then-mutate. The shapes are different, but the underlying flaw is the same:
Any operation with a read followed by a write, without a lock, is race-prone.
The fix isn't "add another check." The fix is either a lock or an atomic database operation. There is no third option.
2. Silent no-ops are more dangerous than loud failures
When save() hits a deleted row, Django doesn't raise. It sends an UPDATE matching zero rows and moves on.
If your code continues past that point — sending a notification, updating a counter, logging an event — you've now built a system that creates side effects for events that never happened.
The only way to catch this class of bug is a concurrency test with realistic ordering.
3. In concurrency tests, choose roles that make the test order-independent
If a test passes in one ordering and fails in another, it's not testing an invariant — it's testing the scheduler. Rewrite the test so the assertion holds regardless of which thread wins. If you can't, the contract isn't fully specified yet.
4. Lock only what you need — and always in the same order
select_for_update on every row you touch seems safe. It isn't. It increases the chance of deadlocks and reduces concurrency for no benefit.
Lock the rows you actually mutate. Read the rest without locking. When you must lock multiple rows, lock them in a consistent order across the codebase.
5. A consistent pattern is worth more than a clever fix
After this chapter, every membership mutation follows the same rule:
WorkspaceService
├── add_member → Nested Atomic + Error Conversion
├── change_member_role → select_for_update
├── remove_member → select_for_update
├── transfer_ownership → select_for_update (already)
└── delete → atomic
A new developer reading this code sees a pattern. A code reviewer spots deviations in seconds. A future bug surfaces faster because the inconsistency stands out.
Consistency isn't glamorous, but it's what makes a codebase survivable.
Section 9: The Results
After two fixes, the full test suite:
test_concurrent_same_role_change ✅ PASSED
test_concurrent_different_role_change ✅ PASSED
test_concurrent_role_change_and_remove ✅ PASSED
Three tests. Two bugs. Two lines of implementation code.
The commits were kept separate, because tests and fixes are different stories:
# Commit 1: Tests
git commit -m "test(workspace): cover concurrent role change scenarios"
# Commit 2: Implementation
git commit -m "fix(workspace): serialize concurrent membership mutations with row locks"
When a future developer bisects these commits, they'll see: first the test that proves the bug, then the fix that closes it. That's how you write a changelog that teaches.
Final Thought
The first race condition taught me that check-then-create is dangerous.
The second one taught me something harder: race conditions are a category, not an incident.
When you find one, don't patch it and move on. Audit the sibling methods. Look for the same shape under a different name. Assume that if you made the mistake once, you made it — or something close to it — everywhere you wrote similar code.
The two bugs in this chapter were hiding in plain sight. They passed every unit test. They passed every integration test. They passed code review. They looked correct.
And under load — under real load, in real production, with real users hitting the same endpoint from different devices — they would have quietly corrupted the state of the system, one duplicate notification at a time.
The fix was two lines.
The lessons were worth much more.
Majid Khazaei is a backend engineer specializing in Django, PostgreSQL, and production-grade API design. He writes about concurrency, data integrity, and the engineering discipline behind reliable systems.
🔗 GitHub: https://github.com/majidkhazaei
🔗 LinkedIn: https://www.linkedin.com/in/majid-khazaei-dev
#django, #python, #concurrency, #backend, #postgresql, #testing
Top comments (0)