Concurrent access to private data

M

Michal

Hi,

maybe it's a simple problem for most of you, but I can't find an easy way to
quite simple problem, let me describe:

Let's have this code:

public ClassToChange()
{
... //some other methods

internal List<AnyDataClass> LockAndGetData()
{
Monitor.Enter(cs);
return data;
}

internal Unlock()
{
Monitor.Exit(cs);
}

private object cs = new object();
private List<AnyDataClass> data = new List<AnyDataClass>();
}

public class Editor
{
public Editor(ClassToChange editedObject)
{
this.editedObject = editedObject;
}

public void MyFunctionToChangeData()
{
List<AnyDataClass> data = editedObject.LockAndGetData();

... // change data

editedObject.Unlock();
}

private ClassToChange editedObject;
}


class Program
{
static void Main()
{
Editor ed = new Editor(new ClassToChange());
ed.MyFunctionToChangeData();
}
}

So my aim is to let Editor change the data of ClassToChange in a thread-safe
way. But as you can see to call all the List data =
editedObject.LockAndGetData(), then not to forget the editedObject.Unlock()
is very boring to write (and not so safe). Do you have any easy solution for
this?

Only solution I have, there might be the CS as attribute of ClassToChange
and use lock {} in Editor, but I would need an attribute to every data object
of ClassToChange, who would check if the cs is locked to make it safe. So
that's also not a good way.

Any ideas?
 
P

Peter Duniho

Michal said:
[...]
So my aim is to let Editor change the data of ClassToChange in a thread-safe
way. But as you can see to call all the List data =
editedObject.LockAndGetData(), then not to forget the editedObject.Unlock()
is very boring to write (and not so safe). Do you have any easy solution for
this?

Only solution I have, there might be the CS as attribute of ClassToChange
and use lock {} in Editor, but I would need an attribute to every data object
of ClassToChange, who would check if the cs is locked to make it safe. So
that's also not a good way.

Any ideas?

If the synchronization needs to happen at the level of operations within
the Editor class, then your synchronization object should also be in the
Editor class.

Of course, this means that the Editor class has to be the de facto owner
of your ClassToChange instance, and no other code should be modifying
the ClassToChange instance while the Editor class is manipulating it.
But that's just good program design anyway. It should not be a problem
to require that.

In some cases, it may be feasible to simply make the ClassToChange class
the thread-safe one. In that case, rather than having the Editor class
try to lock and unlock, just do the synchronization in the ClassToChange
class for each operation that might modify the class's contents. This
would lead to more locking and unlocking, but coherency of the data
would still be preserved.

Finally, if you decide that neither of the above can possibly work (and
I would recommend strongly that you not come to that conclusion…in
particular, in your example it really looks like the synchronization
should be handled entirely by the Editor class, with the ClassToChange
class being left not thread safe), you could implement a "do work"
interface in your ClassToChange class, so that the Editor class simply
passes a delegate that is invoked while the lock is held. For example:

class ClassToChange
{
void DoSynchronizedWork(Action action)
{
lock (cs)
{
action();
}
}

private readonly object cs = new object();
}

class Editor
{
public void MyFunctionToChangeData()
{
editedObject.DoSynchronizedWork(MyFunctionToChangeDataImpl);
}

private void MyFunctionToChangeDataImpl()
{
// change data
}

private ClassToChange editedObject;
}

Hope that helps.

Pete
 
P

Patrice

Hello,
So my aim is to let Editor change the data of ClassToChange in a
thread-safe
way. But as you can see to call all the List data =
editedObject.LockAndGetData(), then not to forget the
editedObject.Unlock()
is very boring to write (and not so safe). Do you have any easy solution
for
this?

Try the SyncLock statement that is intended to simplify Enter/Exit calls.
The doc is at :

http://msdn.microsoft.com/en-us/library/3a86s51t(VS.80).aspx
 
P

Peter Duniho

Patrice said:
Try the SyncLock statement that is intended to simplify Enter/Exit calls.
The doc is at :

http://msdn.microsoft.com/en-us/library/3a86s51t(VS.80).aspx

This is the C# newsgroup. The equivalent is the "lock" statement.
Neither VB.NET's "SyncLock" nor C#'s "lock" offer the semantics that the
OP is asking for (i.e. retaining the lock across method calls).

Granted, as I suggested in my reply to the OP, the semantics being asked
for should be avoided. :) Nevertheless, I don't think that suggesting
"lock" (or "SyncLock", were this a VB.NET newsgroup) would in and of
itself suffice to answer the question.

Pete
 
P

Patrice

This is the C# newsgroup. The equivalent is the "lock" statement. Neither
VB.NET's "SyncLock" nor C#'s "lock" offer the semantics that the OP is
asking for (i.e. retaining the lock across method calls).

Good point. Sometimes I just mix groups.

IMO retaining the lock was not the OP intent and was more caused by the
implementation attempt...
 
M

Michal

Thanks Peter, that's quite interesting solution. But also quite lot of
writing. I know it's best to keep all the code for changing data inside
class. But like this the class will get huge. E.g. imagine class for Bezier
curve - to make all the operations like rotate, move, resize, ... inside it's
awful. I ended up with 3500 lines in one cpp file in one of my projects,
that's not acceptable.

My idea is to provide some kind of "editors" for class, which would change
it. Something similar to MVC pattern maybe(?). Like this the operation is in
once class and it does only what it has to. The only problem is this
synchronization.
 
P

Peter Duniho

Michal said:
Thanks Peter, that's quite interesting solution. But also quite lot of
writing.

It's no more typing ("writing") than your proposed solution of having
separate lock/unlock methods that have to be called while you operate on
an instance. But I agree, it's not the best choice. I simply offer it
only in case you won't or can't make the best choice.
I know it's best to keep all the code for changing data inside
class. But like this the class will get huge. E.g. imagine class for Bezier
curve - to make all the operations like rotate, move, resize, ... inside it's
awful. I ended up with 3500 lines in one cpp file in one of my projects,
that's not acceptable.

In general, classes like that are not thread-safe. Why do you think
yours should be?
My idea is to provide some kind of "editors" for class, which would change
it. Something similar to MVC pattern maybe(?). Like this the operation is in
once class and it does only what it has to. The only problem is this
synchronization.

As I mentioned before, typically you make access to a type thread-safe,
rather than making the type itself thread-safe. Every now and then, you
wind up with a type that is used _only_ in a threaded context, and where
every member or almost every member may be accessed by two or more
threads simultaneously. In those cases, making the type thread-safe
makes sense. But those are rare exceptions.

This is true for at least a couple of reasons: reusable types most
frequently do not need to be thread-safe, so making them thread-safe
causes client code using the class to suffer unneeded overhead when
thread-safe access is not needed; also, as you yourself describe, a
given class may have a large number of mutation points where thread
safety is needed in order to make the whole class thread-safe, but
client code may actually access the instance(s) of that class in only a
small number of places.

Rather than designing classes that are relatively inefficient, one can
limit the implementation of thread-safety to the few places where the
class instance(s) are actually used by client code, saving work and
program overhead.

Pete
 
M

Michal

Thanks for info Patrice, but it's not only about some List<> data, is about
any data class might contain - and Editor class might also require access to
more members. Anyway, I've solved this, but it's far from being nice ...

I've made one class:

public void AddObject(string name, object newObj)
{
objects.Add(name, newObj);
}

public object Get(string name)
{
Debug.Assert(objects.ContainsKey(name));
return objects[name];
}

Dictionary<string, object> objects = new Dictionary<string, object>();

and one interface:

public interface ILockableDataAccess
{
DataAccess LockAndGetData();
void Unlock();
}

So every class with lockable access to data provides LockAndGetData and
Unlock functions and in DataAccess object it returns required objects. It's
boring, not safe and requires lot of TYPING (sorry for my English in last
post) and converting between object and my types. But only solution I have
now ...

To answer Peter's question:yours should be?

Generally my program constantly generates some graphical data for other
input devices in separate thread and it allows user to change them anytime.
So I need thread safe access to edit and generate methods of different
classes.
 
M

Michal

Sorry, here is the DataAccess class complete:

public class DataAccess
{
public void AddObject(string name, object newObj)
{
objects.Add(name, newObj);
}

public object Get(string name)
{
Debug.Assert(objects.ContainsKey(name));
return objects[name];
}

Dictionary<string, object> objects =
new Dictionary<string, object>();
}
 
P

Peter Duniho

Michal said:
[...]
To answer Peter's question:yours should be?

Generally my program constantly generates some graphical data for other
input devices in separate thread and it allows user to change them anytime.
So I need thread safe access to edit and generate methods of different
classes.

Non-sequitur. The fact that you need thread-safe access to the data
doesn't mean that the data types themselves need to be thread-safe. The
List<T>, Queue<T>, and many other .NET classes are not thread-safe, and
yet I use them in a thread-safe way in multi-threaded code on a regular
basis. The same thing can be true for your own types.

That said, you seem set on doing things the hard way, so I suppose that
will have to do.

Pete
 
M

Michal

Peter, I don't think it's safe to e.g. add some item in one thread to
List<Whatever> and to read all the items from this List<Whatever> in another
thread (that's what I do). So I'm sure, it's required to use thread safe
access to my ClassToChange.
 
P

Peter Duniho

Michal said:
Peter, I don't think it's safe to e.g. add some item in one thread to
List<Whatever> and to read all the items from this List<Whatever> in another
thread (that's what I do). So I'm sure, it's required to use thread safe
access to my ClassToChange.

No one, least of all me, is disputing that. The question is whether you
have a need to build thread-safety into the object itself.

The problem you are asking for help with is a consequence of your
apparent desire to have a specific class manage the object used for
locking: the class that you want to operate on in a thread-safe manner.

But, if you would simply forget trying to make that class thread-safe,
and instead require that clients of the class access it with their own
thread-safe behavior, the problem you're asking for help with just goes
away. The "lock" statement becomes completely sufficient for the need.

Pete
 
M

Michal

Yes, I see your point and this discussion is really helping me. But I use
this class in two different threads - one is changing it and second is using
it... Anyway maybe you think about using it like this:
....
lock (editedObject)
{
//change it
}

and in my second thread also
lock (editedObject)
{
//use it
}

Yes, that might be possible - but also when I create attribute for the
List<AnyDataClass> data e.g.:

List<AnyDataClass> Data { get { return data } }

But there's no way to assure object is locked, when accessing this data, is
there?
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top