Skip to content

Comprehensive Code Audit: 24 findings (3 CRITICAL — mutual exclusion broken) #13

@tsavo-at-pieces

Description

@tsavo-at-pieces

Summary

A comprehensive code audit of named_locks (v1.0.0-beta.5) identified 24 issues. The most severe finding is that mutual exclusion is not reliably provided due to compounding bugs in the lock lifecycle.

Severity Count
CRITICAL 3
HIGH 7
MEDIUM 6
LOW 8

CRITICAL (3)

C1. Lock resource leak on exception during critical section

  • File: lib/src/named_lock.dart, lines 259-282
  • No try/finally around execute + cleanup. If execution.execute() throws, lock.unlock(), lock.close(), and lock.unlink() are all skipped. OS semaphore remains locked permanently.
  • Fix: Wrap execute-cleanup in try/finally

C2. Timeout mechanism is completely ineffective

  • File: lib/src/named_lock.dart, line 252
  • lock.lock() calls sem_wait() (blocking), freezing the entire Dart isolate. The timeout, backoff, and sleep are all dead code.
  • Fix: Use lock.lock(blocking: false) (calls sem_trywait) for the retry loop

C3. Async callables — lock released before Future resolves

  • File: lib/src/named_lock.dart:259, lib/src/execution_call.dart:135-155
  • When callable returns Future<R>, execute() stores unresolved Future and returns immediately. Lock released while async work still running.
  • Fix: Detect Future returns and await completion before cleanup, or throw on async callables

HIGH (7)

  • H1: NativeSemaphores.register() inverted assertion — guard never fires
  • H2: Unbounded memory leak in static registries — entries never deleted after unlink()
  • H3: Non-blocking lock failure THROWS instead of returning false
  • H4: Cleanup return values silently discarded — leaking OS resources
  • H5: sleep() blocks entire isolate during backoff (up to 500ms)
  • H6: unlock() short-circuits via ||sem_post never called, OS semaphore never released. CRITICAL severity.
  • H7: _throwStoredError null dereference replaces original error with TypeError

MEDIUM (6)

  • M1: Timeout overshoot by up to 500ms
  • M2: Non-deterministic hashCodes in CapturedCallFrame.caller
  • M3: Static registries are isolate-local (counter stats misleading)
  • M4: late final reassignment may throw on singleton reuse
  • M5: Lock name validation missing
  • M6: waiting message unreachable (dead feature)

LOW (8)

  • L1-L8: Unused typedef, mutable static prefix, redundant defaults, missing @OverRide, verbose print(), fragile guarding, non-deterministic test sleeps, misleading backoff value

Root Cause

The issues compound: lock.lock() blocks forever (C2) -> timeout dead code -> unlock() short-circuits (H6) -> close()/unlink() fail silently (H4) -> singleton reuse skips OS locking (M4). After the first guard() call, mutual exclusion is broken.

Missing Tests

Category Risk
Lock timeout enforcement CRITICAL
Mutual exclusion verification CRITICAL
Lock release on exception CRITICAL
Async callable behavior HIGH
Second-call singleton reuse HIGH

Priority

  1. C2+H6: Restore actual locking/unlocking | 2. C1: try/finally | 3. C3: Async detection | 4. H4: Check cleanup returns

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions