Skip to content

Fixes deadlock in StateLockComponent + MovementLockComponent#359

Closed
kenedos wants to merge 1 commit intoNoCode-NoLife:masterfrom
kenedos:deadlock_fix1
Closed

Fixes deadlock in StateLockComponent + MovementLockComponent#359
kenedos wants to merge 1 commit intoNoCode-NoLife:masterfrom
kenedos:deadlock_fix1

Conversation

@kenedos
Copy link
Contributor

@kenedos kenedos commented Feb 14, 2026

This PR fixes the following issue:

The Deadlock Explained

  There are two locks involved on two different components of the same entity:

  1. StateLockComponent._syncLock — protects lock/state dictionaries
  2. MovementComponent._positionSyncLock — protects position data

  The deadlock is a classic lock ordering inversion:

  Thread A — Map heartbeat (runs every tick)

  MovementComponent.UpdateMove()
    lock(_positionSyncLock)          ← acquires position lock
      → updates position
      → calls Entity.IsLocked(LockType.Movement)
        → StateLockComponent.IsLocked()
          lock(_syncLock)            ← WAITS for state lock

  Thread B — Skill/combat (any skill applying stun, freeze, knockback, etc.)

  StateLockComponent.AddState("Stunned")
    lock(_syncLock)                  ← acquires state lock
      → calls this.Lock("Movement")
        → Lock() re-enters _syncLock (reentrant, succeeds)
        → OUTSIDE the inner lock but INSIDE the outer lock:
          → calls movement.Stop()
            lock(_positionSyncLock)   ← WAITS for position lock

  Thread A holds _positionSyncLock, waits for _syncLock.
  Thread B holds _syncLock, waits for _positionSyncLock.
  Neither can proceed — deadlock.

Steps to Reproduce (before fix):

  1. Add the following on line 456 of MovementComponent.cs to facilitate the deadlock:
    System.Threading.Thread.Sleep(500);
  2. Be moving around as an entity stuns you -> Server deadlocks
  3. Preferably cause Stun inside a CallSafe(this.Attack(...));, as this is guaranteed to run in a separate thread.

Notes:

  • Melia does not currently have any mobs that cause state locks and PvP does not exist yet, so this issue cannot happen as of today. But this is guaranteed to be an issue as mobs gain their own skill handlers and/or PvP systems are added.

@kenedos kenedos closed this Feb 17, 2026
@kenedos
Copy link
Contributor Author

kenedos commented Feb 17, 2026

Tested, this deadlock does not actually exist in Melia as MovementComponent.Stop() does not lock the entities' pathfinding nodes to clear them up. This may cause other issues such as entities not having the correct pathfinding nodes after getting knocked back/down (i.e. they will have incorrect positions on the _path queue), but this PR makes no sense to be merged as it is, due the deadlock issue itself not actually existing in the codebase.

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.

1 participant