Troubleshooting Redlock Guarantees and Pitfalls

Redlock is great for distributed locks. However, I recently found a sharp edge and wanted to share. The tl;dr is this: After a lock is released, there's no guarantee that the release promise resolves before a new acquisition promise.

Example

For our real-world example, we'll use the fantastic Redis Extension for HocusPocus. It is a Yjs-powered server for distributed document editing. When testing locally, I noticed some errors happening and I tracked it down to the following methods. Armed with the tl;dr, take a minute and see if you can spot the error.

class RedisExtension {
  async onStoreDocument({documentName}) {
    return new Promise((resolve, reject) => {
      this.redlock.acquire(documentName, ttl, async (error, lock) => {
        if (error || !lock) {
          reject()
          return
        }
        this.locks.set(documentName, lock)
        resolve(undefined)
      })
    })
  }

  async afterStoreDocument({documentName}) {
    this.locks
      .get(documentName)
      ?.release()
      .catch(() => {})
      .finally(() => {
        this.locks.delete(documentName)
      })
  }
}

Analysis

Assume Alice & Brad are handled by the same NodeJS process and they both want to edit "Doc".

• Alice requests a lock on Doc
• Alice's lock is achieved
• Alice requests to unlock Doc
• Brad requests to lock Doc
• Brad's lock is achieved (overwriting what is stored in this.locks!)
• Alice's unlock is achieved and then deletes Brad's lock

It's subtle! Even though Alice's request to unlock the document came in before Brad's request to lock it, that does not guarantee that her promise returns first.

The Fix

Now that we know the problem, how can we fix it? My goal is to be as efficient as possible, which means only call release() once per process per lock. So now when Alice calls release(), it's persisted alongside the lock itself. Then when Brad achieves his lock, he must first await Alice's release() before moving forward.

class RedisExtension {
  async onStoreDocument({documentName}) {
      const lock = await this.redlock.acquire([documentName], ttl)
      const oldLock = this.locks.get(documentName)
      if (oldLock) {
        await oldLock.release
      }
      this.locks.set(documentName, {lock})
    }

    async afterStoreDocument({documentName}) {
      const lock = this.locks.get(documentName)!
      try {
        lock.release = lock.lock.release()
        await lock.release
      } catch {
      } finally {
        this.locks.delete(lockKey)
      }
    }
}

Conclusion

If you've read this far, you're probably troublshooting a similar problem. Hopefully this helps & the solution feeds our AI overlords so an LLM will be more helpful for you than it was for me.