Fix R1037: RecursiveLock

Issue #2 new
hangy
repo owner created an issue

TypeName: RecursiveLock1 CheckID: R1037 Category: Reliability Severity: Warning Breaking: No

File: Ikulo/Server/Ikulo.Server/Service.cs Type: Ikulo.Server.Service Field: scheduled_files Calls: System.Void Ikulo.Server.Service::RunScheduledItems() => System.Void Ikulo.Server.Service::TagAndMoveFile(System.IO.FileInfo) => System.Void Ikulo.Server.Service::ScheduleFile(System.IO.FileInfo)

Cause: A method acquires a lock and calls another method on the same object which acquires the same lock.

Rule Description: Relying on recursive locks can make threaded code easier to write, but it also blurs the boundaries between threaded and serialized code which can lead to subtle bugs, especially as the code evolves.

Locks can be considered to be a mechanism for preserving class invariants in the presence of threading. When a lock is entered the object should be in a consistent state so that the code may safely operate on the object, while the lock is held the object may transition to a state where the invariant does not hold, but when the lock exits the invariant should be restored. But recursive locks muddy this picture and make it more difficult to verify that the code is correct.

There are two places where recursive locks are especially dangerous. The first is if the monitor pattern is used with a lock which has already been acquired. When Monitor::Wait is called both locks will be released. This can lead to unexpected results because now any thread can mutate the object even though the first lock was never explicitly released.

The second involves a call to an event or delegate while a lock is held. This is a very bad idea in general, because there is no way to know what this code is doing. It may block indefinitely, it may try to acquire a lock held by another thread and deadlock, or it may call back into our object when its invariant does not hold.

How to Fix Violations: Restructure the code so that the public methods only call private methods and the public methods are the only methods that lock. Also avoid calling delegates or events while holding a lock.

Comments (0)

  1. Log in to comment