2

I want to make update operations to the DataTable/DataSet thread-safe. There are ~20 threads each updating ~40 rows of global DataTable using Rows.Find(pk) method of DataTable. Each thread will be updating distinct rows of DataTable.

I'm using the following wrapper class for DataSet. Is this approach thread-safe?

public sealed class MyDataSet{

    public static DataSet ds = new DataSet();

    public static UpdateRow(key,data)
    {
        object _lock = new object();
        DataRow dr = ds.Tables[0].Rows.Find(key);
        lock(_lock){          
            dr.AcceptChanges();
            dr.BeginEdit();
            dr["col"] = data;
            dr.EndEdit();
        }
    }
}

This method is called from a for loop.

for(int x=0; x<40; x++;){
    if(someCondition)
    .
    .
    .
    MyDataSet.UpdateRow(key,data);
    .
    .
    .
    }

Everything is done in a multithreaded environment. Is UpdateRow method thread safe?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
techknowfreak
  • 259
  • 4
  • 18

2 Answers2

6

No, it is not safe. You should change your code in:

public sealed class MyDataSet{

    public static DataSet ds = new DataSet();

    private static object _lock = new object();

    public static UpdateRow(key,data)
    {
        lock(_lock){
            DataRow dr = ds.Tables[0].Rows.Find(key);
            dr.AcceptChanges();
            dr.BeginEdit();
            dr["col"] = data;
            dr.EndEdit();
        }
    }
}

Your _lock object should be a static object in your program to make it a good lock. And your Find should be in the locked part.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Peter
  • 27,590
  • 8
  • 64
  • 84
1

Is UpdateRow method thread safe?

No it's not. You are creating a lock on a new object every time you enter the method. The object on which you lock has to be the same across all threads - otherwise the lock will never be considered "taken" since each thread happily creates a new lock on an object it throws away afterwards. One way to achieve this would be making the object you lock on static as well.

BrokenGlass
  • 158,293
  • 28
  • 286
  • 335