zoidbeck
zoidbeck

Reputation: 4151

Publicly visible lock object: Sometimes useful or indication for design flaw?

I'm monitoring a subcontracted supply of a software component managing some low level network communications. Reviewing this component i found this:

public static object SyncRoot { get { return syncRoot; } }

Within this very essential class nearly any method uses this lock object while doing things. Now i wonder if this is good decision to implement such a bottleneck or is it after all a serious design flaw. Isn't this some weird kind of multithreading the procedural way?

Besides the problem of possible deadlocking is locking quick? How high is the monitoring efford to manage all this concurrend threads? Wouldn't it be better (if somehow necessary) to implement something like a synchronous Pause/Resume-Method to get this class stop/start working from outside? In what situation is a publicly visible lock-object usefull? Is there any?

Upvotes: 2

Views: 226

Answers (3)

phoog
phoog

Reputation: 43046

Public SyncRoot is used in the MS framework, it is true, but that is an instance property, not a static property. You don't give any detail about the scope or accessibility of the syncRoot object returned by the (Static) SyncRoot property in your example. The documentation (http://msdn.microsoft.com/en-us/library/ms173179.aspx) warns against locking on a public type, or an object "beyond the control of your application". There's a very good discussion of multithreading at http://www.albahari.com/threading/.

Upvotes: 2

Steven
Steven

Reputation: 172646

The SyncLock is essentially a design flaw in the .NET framework. For this reason the .NET 2.0 collections don't expose this property directly (it is implemented explicitly and needed for backward compatibility). The problem is that locking is often to granular, locking is happening at a level that is too low. Usually this problem can be solved by locking at a higher level component, that controls the collection.

Using the SyncRoot property there are indeed changes of having deadlocks, because you have no control over how consumers use that locked object. They might also take a lock for a very long time, or forget to unlock it. The idea of Pause/Resume sounds very scary.

The usual thing to do is to make the component thread-safe. When you do this, you won't need the SyncRoot property.

Upvotes: 1

Dean Chalk
Dean Chalk

Reputation: 20461

this is often used for collection synchronisation within the MS framework

http://msdn.microsoft.com/en-us/library/system.collections.icollection.syncroot.aspx

Upvotes: 0

Related Questions