1

I have a reasonably simple case of two threads interacting with the same data structure. The threads are hosted in their own responsible classes. Let's say these are class Alfons and class Belzebub:

class Alfons { public Mutex listMutex = new Mutex(); private void ProcessListInfo() { listMutex.WaitOne(); // // ... Process multi-access list stuff ... // listMutex.ReleaseMutex(); } } class Belzebub { private Alfons mCachedAlfonsReference; private void ProcessListInfoDifferently() { mCachedAlfonsReference.listMutex.WaitOne(); // // ... Process multi-access list stuff in a different fashion ... // mCachedAlfonsReference.listMutex.ReleaseMutex(); } } 

My question is whether referencing a Mutex like this can create a concurrency issue OR whether it is recommended practice to do so. Is there a better way of doing this and should I, for example, cache the mutex reference rather than accessing it through a reference.

2
  • Yes! Thank you very much! I would, in fact, like to mark you both as answer (dcastro & acarlon). I ended up doing some restructuring on my datatype thanks to these :) Commented Jan 22, 2014 at 20:32
  • Glad to help - dcastro was first, so fair enough. Commented Jan 22, 2014 at 20:36

3 Answers 3

2

There would be no concurrency issue - the mutex is supposed to be shared. As per the Mutex MSDN docs

This type is thread safe.

However, I'd say that the data structure itself should synchronize access coming from different threads. If the data structure doesn't support this (e.g., using SyncRoot), encapsulate it and add that feature.

Out of curiosity: which data structure are you using? You might consider using one of the System.Collections.Concurrent collections for lock-free/fine-grained locking solutions. Also, wouldn't using the lock keyword be simpler and less error-prone for your scenario?

Sign up to request clarification or add additional context in comments.

Comments

2

Generally, since locking can be tricky and deadlocks will stop all fun, I try to reduce the code that is concerned with the mutex rather than passing it around. Otherwise it can be a headache to figure out which paths lead to a lock.

It may be better to encapsulate the resource and thread critical operations in a class and then:

Lock( this ) { } 

Or see if there is a thread-safe version as suggested by dcastro.

Besides this, be very careful that there is no return (throw, etc) between WaitOne() and ReleaseMutex() otherwise other threads will be locked out indefinitely - lock or a finally with the ReleaseMutex is safer in this respect. As castro pointed out in the comments, it could be another library that raises the exception.

Finally, I am assuming that it is the same resource that is being protected in ProcessListInfo() and ProcessListInfoDifferently(). If these are two different resources that are being protected, then you have extended the likelihood of unnecessary thread contention.

1 Comment

+1, the mutex should be released within a finally block, to guard against possible exceptions. Even if your own code doesn't throw one intentionally, be reminded that exceptions like ThreadAbortException can still occur.
1

I don't see how caching the mutex reference would make any difference, either way you are still accessing the same object through references, and if you don't do that then it defeats the point of a mutex.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.