views:

132

answers:

4
abstract class Foo
{
    private List<Object> container;
    private bool update;

    Foo Foo()
    {
        container = new List<object>();
        update = false;
    }

    public abstract Bar CreateBar();

    public void BeginUpdate()
    {
        if (!update)
        {
            Thread update_thread = new Thread(new ThreadStart(Update));
            update_thread.Start();
        }
    }

    private void Update()
    {
        update = true;
        while (update)
        {
            lock (container)
            {
                if (...)
                    container.Add(this.CreateBar());
                else
                    container.Remove(...);
            }

            Thread.Sleep(1337);
        }
    }

    public void EndUpdate()
    {
        update = false;
    }

    public List<Object> Objects
    {
        get
        {
            lock (container)
            {
                return this.container;
            }
        }
    }
}

When something outside of Foo calls the Foo's Object accessor like,

List<Objects> objects = foo_instance.Objects;
foreach (Object o in objects)
{
    Thread.Sleep(31173);
}

How will the locking occur? Will the thread running Update() have to wait until the above foreach is done processing objects list? I would like that these two would work simultaneously, is the only solution to make a deep copy of objects?

+1  A: 

You can think of lock scope similar to function scope. Doing a lock inside of your accessor method will only lock between the braces, once the value is returned the lock will be released. You'll need to do the locking outside of the class (in the caller) to get the desired effect.

So both the loop and the thread will be able to access objects at the same time. That's bad, because if your thread changes objects during the loop, the loop will throw an exception because the collection was modified.

SoapBox
+2  A: 

Your code does not do what you think it does. This method

public List<Object> Objects
{
    get
    {
        lock (container)
        {
            return this.container;
        }
    }
}

Does not hold the lock after it has returned the value. So your loop is not locked.

You cannot return the container instance from the clas

krosenvold
+1  A: 

Several problems with this code:

  1. You do not start the thread
  2. You can have a race-condition (might not be applicable to your program) where multiple threads call BeginUpdate, both sees that update is false, and both start a thread, now you have two threads running, possibly interfering with each other, since you have a field member with the shared data
  3. What is the point of locking in the property? You don't need it. Note that the container you return from the property will at times be in a state of flux due to the inner update. This means that outside code that calls the property will have to lock the container itself before accessing its contents. You do not need a lock to return the container reference though.
  4. Race condition if you call EndUpdate and then BeginUpdate right after each other, the old thread might not have had a chance to exit yet

I'd say that before you start worrying about what to lock and when, you should fix the code so that it doesn't have lots of other thread-related problems.

Lasse V. Karlsen
Thanks, good insight. Had to give you a verbal cherry since I cannot vote. ;>
A: 

krosenvald is correct, the lock on the Objects accessor is released as soon as the property returns the pointer to the container object...

In you code

List<Objects> objects = foo_instance.Objects;
foreach (Object o in objects)
{    
    Thread.Sleep(31173);
}

The lock only lasts for the first line... where the reference variable "objects" is being populated... There's really no need to lock anything for that line since no memory is being modified by fetching the pointer...

Charles Bretana
Would there be a possible access violation if object 'o' was used and accessed inside the foreach?
Well, what do you mean by "access violation"? There certainly will not be any lock contention, as the target of the lock will have released the locj by the time the foreach begins...
Charles Bretana