C#: datatable thread safety questions

I have a singleton obj called DataStorage that sto开发者_如何学运维res common data in my app; one of these is a Datatable called myTable that will be read and written by multiple threads. I have a private object to be used as a lock in DataStorage i.e.

private object lockObj = new object();

I have wrapped locks around accessing myTable as such:

private DataTable myTable;
public DataTable MyTable
{
    get
    { lock(lockObj) { return myTable; } }
    set
    { lock(lockObj) { myTable = value; } }
}

Another object i.e. MyObj will get this datatable, do a Select on it and then modify some value in the retrieved DataRow[] from Select. I have read that Select is not thread-safe, so I wrote my code like this:

// lock on MyTable
DataTable dt = DataStorage.Instance.MyTable;

lock (MyObjLockObj)  // lock object for MyObj class
{
    // do a select, then modify value in the returned row
    DataRow[] foundRows = dt.Select("some expression");
    foundRows[0]["some col"] = 123456;
}

Questions: 1. Overall, is this code thread-safe?

1. When I modify the retrieved DataRow in MyObj, is the MyTable setter ensuring its thread-safety? Because the setter is for setting DataTable, not a DataRow.

2. Should I move the code accessing DataStorage.Instance.MyTable into the same lock block as the Select?

Thanks in advance.

you code isnt thread safe, because:

the lock on the property only makes sure more that one thread cant use the get / set property at the same time, but after they have access they can change it at the same time.

i.e.

a thread can use the MyTable property to get in instacne of the datatable. after that another thred can use the property to get that same instance. then after they both have access to myTable they can both write to it directly with no synchronization.

so moving the accessor into a lock wont help either.

its better to provide methods that change the data , and do the sync in those methods, and not expose the datatable directly.

Those locks in getter and setter are totally useless. 'myTable = value' operation is already atomic, no need to wrap it in a lock. And this lock has nothing to do with modifying rows, of course. That lock (MyObjLockObj) however ensures that one and only one thread will access your datatable, if same MyObjLockObj is used though your app. If you're using that datatable infrequently, it won't be a problem, however if your app is all about messing with that datatable, you'll get performance issues.

All in all, using a single datatable in a multi-threaded application is a bad decision. If you read data for that datatable from the DB, you should read one datatable per thread, modify its rows and commit changes back to the DB.