C# using lock practice

Question

I have critical section in my application which contains a lot of code: which is better way to locking access in threadMethod:

A) lock all block:

private object locker = new object();

private void threadMethod()
{
   while(true)
   {
      lock(locker)
      {
         // do work - a lot of code here
      }
      Thread.Sleep(2000);
   }
}

B) Use additional locked access member canWork:

 private static object locker = new object();
 private bool canWork;
 private bool CanWork
 { 
     get { lock(locker) { return this.canWork; } }
     set { lock(locker) { this.canWork = value; } } 
 }

  private void threadMethod()
  {
     while(true)
     {
        if(CanWork)
        {
           // do work - a lot of code here
        }
        Thread.Sleep(2000);
     }
  }

and somewhere in code

CanWork = false;

The two chunks of code have completely different semantics. Tell us more about what you're trying to do and we'll tell you which chunk, if any, does what you need. — LukeH
– LukeH, Commented Nov 24, 2010 at 12:02
Having a lock statement in property setters/getters seems like a recipe for disaster. Also check if you really need to look it entirely or if you can get away with something like ReaderWriterLockSlim — Homde
– Homde, Commented Nov 24, 2010 at 12:04
@MattiasK - I don't see why this is 'recipe for disaster' - can you explain? — Przemysław Michalski
– Przemysław Michalski, Commented Nov 24, 2010 at 12:10

Mark Byers · Accepted Answer · 2010-11-24 12:18:36Z

4

Neither is particularly good.

The first has the disadvantage that you hold the lock for a long time.
The second has the disadvantage that the state can change after you check it.

Instead try to pass immutable arguments to your method (for example a copy of the data). You will probably still need to lock for constructing the arguments and for collecting the results but this is hopefully a much shorter period of time.

edited Nov 24, 2010 at 12:18

answered Nov 24, 2010 at 12:02

Mark Byers

843k202 gold badges1.6k silver badges1.5k bronze badges

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

5 Comments

Przemysław Michalski Over a year ago

In the second case, when state of "CanWork" member will change, work will be done while next "while" loop - where do you see disadvantages?

Mark Byers Over a year ago

@UGEEN: The boolean could change to false after you read the value but before you actually start working on it (perhaps because another thread started working on it at exactly the same time). In this case you could end up with two threads doing the same work, and possibly interfering with each other's work.

Przemysław Michalski Over a year ago

boolean is locked - do you mean the situation which occurs directly after boolean state read, but before 'while' condition is checked?

Mark Byers Over a year ago

@UGEEN: I mean that the value of the boolean can change after it is read but before the if statement is evaluated.

Przemysław Michalski Over a year ago

So there is no easy way - thank you for your time and response.

alpha-mouse · Accepted Answer · 2010-11-24 12:02:03Z

1

The second approach will likely lead to race conditions. Can your "a lot of code" be separated in several critical/non critical chunks?

answered Nov 24, 2010 at 12:02

alpha-mouse

5,01328 silver badges38 bronze badges

1 Comment

Przemysław Michalski Over a year ago

yes, but I tried to avoid this to keep code readability - now I'm increasingly convinced, that it will be difficult

Phil Baines · Accepted Answer · 2010-11-24 12:43:59Z

1

I would use the Monitor instead. Plus do you really want while(true) because this will repeat forever?

  private object syncObject = new object();

  private void threadMethod()
  {
   bool tryToRun = true;
   while(tryToRun)
   {
    if(Monitor.TryEnter(syncObject))
    {
     tryToRun = false;

     // do work - a lot of code here

     Monitor.Exit(syncObject);
    }
    else
    {
     Thread.Sleep(2000); // Possibly knock this up the how long you expect the lock to be held for.
    }
   }
  }

answered Nov 24, 2010 at 12:43

Phil Baines

2054 silver badges11 bronze badges

2 Comments

Przemysław Michalski Over a year ago

is not Monitor a less smart lock?

Phil Baines Over a year ago

From my understanding Monitor.TryEnter() is better because it doesn't stall you're application waiting to get a lock. It simply returns false if another thread has the lock. You can then try again later in your own time.

Eric Ouellet · Accepted Answer · 2011-06-14 18:22:18Z

0

est link:

http://msdn.microsoft.com/en-us/magazine/cc188793.aspx#fig7

Best usage is - declare a new private sync object - use "lock(synObject) { code here ... }

answered Jun 14, 2011 at 18:22

Eric Ouellet

12k15 gold badges102 silver badges135 bronze badges

Collectives™ on Stack Overflow

C# using lock practice

4 Answers 4

5 Comments

1 Comment

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

5 Comments

1 Comment

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related