none
TableAdapter thread safety RRS feed

  • Question

  • Greetings,

    I have an application that has a single TableAdapter which acts as the interface between a SQL Server table and the related DataSet object.

    So that database updates don't slow the execution of my application, I have the calls to TableAdapter.Update() on a separate thread.  Whenever an action changes the underlying DataSet, I trigger the thread to make a call to TableAdapter.Update().

    Is this thread-safe?  Meaning, what happens if the DataSet is modified in the middle of the TableAdapter.Update() method? 

    Thanks,
    Rick
    Tuesday, March 18, 2008 4:48 PM

Answers

  • You're still writing data across threads with this approach.  Frankly, I'm surprised it works at all; I didn't know that table adapters had an overload of Update that accepted a List.

     

    Edit:  I hate, hate, hate the "feature-rich" forum editor that we have here.  The last word of that sentence should be List left-angle-bracket DataTable right-angle-bracket.

     

    Remember that the Update method calls AcceptChanges on every DataRow that it updates the database with.  If you just pass a DataRow object from your UI thread to your background thread and then pass that object to Update, you haven't fixed the cross-threading problem.  This line of code:

     

    Code Snippet

    rowArray = m_NewRows.ToArray();

     

    is creating a new array, but the DataRow objects it contains are the same ones that were in the old one.

     

    I suggested that you use the DataRows in the List to populate a new DataTable, and then pass that DataTable to the Update method.  You'll populate the DataRows' ItemArray with the ItemArray from the original DataRow.  Yes, this means that you'll still be using the same objects across threads.  But access to the objects in the specific columns is genuinely read-only, so that shouldn't cause a problem.

     

    I don't know that this is what's causing the problem in the UI that you're observing, but it could be.

     

    Also, your strongly-typed DataTable should in fact contain a method that lets you specify all of the columns for a row when you add it.  There should be two overloads of the Add[TableName]Row method.  One of them takes a [TableName]Row object, and the other's argument list should map onto the table's mutable fields.

    Wednesday, March 19, 2008 7:06 PM

All replies

  • It's not thread-safe.  The table adapter's Update() method calls AcceptChanges() on the DataSet, and that's a write operation. 

     

    As to "what happens," well, it's hard to say, because that's kind of the nature of code that's not thread-safe.  I would suspect that the range of outcomes goes from perfectly fine to the beloved internal error 5.

    Tuesday, March 18, 2008 7:53 PM
  • Robert,

    Thanks for your response(s).  I'm trying to figure out a way to write to the SQL Server database and not have it slow down the execution of the application (or at least negligibly).  Rows are added to this table at a very high rate, and it seems like you're telling me that -- since TableAdapter.Update() is not thread-safe -- I'll have to lock all write operations to the DataTable and lock the Update() call on the TableAdapter.  This will be not much different, from a speed perspective, as making inline calls to Update() whenever I update the table (or to skip this step altogether and use the TableAdapter to add/remove rows to the table).  In other words, a database update for every DataTable update. 

    Am I right in this assumption?  Or is there a way to have writes to the DataTable occur asynchronously with database updates?  I'd really like to use the DataSet/Table and TableAdapters because they are very powerful (and make my life a lot easier) but it looks like I'm going to have to do some producer/consumer model and have some custom class that represents each row.  One thread would place one of these "rows" into a queue whenever transactions occur; the other thread -- the consumer -- would dequeue items and write them to the database.

    Rick
    Tuesday, March 18, 2008 11:20 PM
  • I don't think you necessarily have to go so far as to make a custom class.  For instance, your update code could look as simple as this:

     

    Code Snippet

    DataTable t = null;

    lock (m_Table)

    {

       t = m_Table.Copy();

       m_Table.AcceptChanges();

    }

    myTableAdapter.Update(t);

     

    Your insert operation will block and wait long enough for this method to make an in-memory copy of the DataTable and call AcceptChanges() on the original.  That won't be instantaneous, but it'll be a lot faster than waiting for the database.

     

    Are you using the original for anything except adding rows?  If not, replace that AcceptChanges() with Rows.Clear().  Otherwise, you're going to end up with a different problem, which is that making the copy will take longer and longer every time you do it.  You can even get around that, though:

     

    Code Snippet

    DataTable t = m_Table.Clone();

    lock (m_Table)

    {

       foreach (DataRow r in m_Table)

       {

          if (r.RowState == DataRowState.Added)

          {

             DataRow r1 = t.NewRow();

             r1.ItemArray = r.ItemArray;

             r.AcceptChanges();

          }

       }

    }

    myTableAdapter.Update(t);

     

    To get any more velocity than that, you'd need to add the rows to a List<DataRow> at the same time you added them to the table, and then copy the rows in that list into your temporary table inside the lock block.

    Wednesday, March 19, 2008 1:41 AM
  • Robert,

    You hit the nail on the head -- unfortunately these tables can grow well beyond 600,000 rows, so the copy procedure would start to be prohibitively slow.  I like the latter approach, however (maintaining a List<DataRow> collection).  How does this approach seem to you -- at the point where the items are added to the List:

    Code Snippet

    MyCustomDataSet.MyCustomDataRow row = MyCustomDataSet.MyCustomDataTable.NewCustomDataRow();
    row.CorrelationID = e.GetCorrelationID();
    ... // I really wish there was a row constructor that took in all the fields!!

    MyCustomDataSet.MyCustomDataTable.AddCustomDataRow(row);


    lock (m_NewRows) {   // m_NewRows is a List<DataRow> object
        m_NewRows.Add(row);
    }
    m_RowModificationSyncEvents.NewItemEvent.Set();


    So I actually add the DataRow to the DataTable (because I have UI elements that depend on these DataTables, and if they are added in a thread other than the main UI thread, the UI elements don't update automatically when rows are added or changed in the DataTable).  I then tell the other thread that there's a new DataRow.

    In the other thread, the consumer, I do the following:

    Code Snippet
    DataRow[] rowArray = null;
    lock (m_NewRows) {
        rowArray = m_NewRows.ToArray();
    }
    m_TableAdapter.Update(rowArray);


    So the thread only locks the List long enough to call ToArray().  Since I know the main thread will never *modify* a row in this particular table (it only adds to the table), I just call Update() on this DataRow[] outside of the lock.

    This seems like a viable for the DataTables that are only ever appended to, however for the tables that are modified, I think I'm going to have to go with the lock-the-table-and-clone approach, but fortunately these tables are not very large in size, so this hopefully won't be too long of an operation.

    Does the above seem like a solid approach to you?  Thanks again.
    Wednesday, March 19, 2008 3:15 PM
  • Ok, so I just implemented the method described above, and it has resulted in some weird behavior -- I'm hoping someone can tell me what this is all about:

    I have a DataGridView that is bound to my DataTable -- it's great: usually, whenever changes are made to the DataTable, those changes are automatically reflected in the DataGridView.  These changes, however, MUST be made on the main UI thread, otherwise, the changes are not automatically reflected in the DGV.

    What I'm seeing having implemented the method above is that sometimes these changes are not reflected in the DGV (it seems random, but once whatever it is happens, changes are NEVER reflected in the DGV -- it's as if some link is broken). 

    This is strange because the changes to the DataSet ARE made on the main UI thread:

    Code Snippet

    MyCustomDataSet.MyCustomDataRow row = MyCustomDataSet.MyCustomDataTable.NewCustomDataRow();


    // this next line is occuring in the main UI thread, so the DGV should update?

    MyCustomDataSet.MyCustomDataTable.AddCustomDataRow(row);


    lock (m_NewRows) {   // m_NewRows is a List<DataRow> object
        m_NewRows.Add(row);
    }



    And the changes to the database are made on a separate thread:

    Code Snippet

    DataRow[] rowArray = null;
    lock (m_NewRows) {
        rowArray = m_NewRows.ToArray();
    }
    m_TableAdapter.Update(rowArray);



    So I might have 10 or 20 successful occurences, and then suddenly one of the rows doesn't show up in the opened DGV (it IS properly adding to the DataTable and the database, it's just that the link between the DataTable and the DGV is broken).

    No exceptions are being thrown, etc., so I can't figure out where that link is being broken.  Is there a way to re-establish this link so that the DGV shows the complete DataSet?


    Wednesday, March 19, 2008 5:33 PM
  • You're still writing data across threads with this approach.  Frankly, I'm surprised it works at all; I didn't know that table adapters had an overload of Update that accepted a List.

     

    Edit:  I hate, hate, hate the "feature-rich" forum editor that we have here.  The last word of that sentence should be List left-angle-bracket DataTable right-angle-bracket.

     

    Remember that the Update method calls AcceptChanges on every DataRow that it updates the database with.  If you just pass a DataRow object from your UI thread to your background thread and then pass that object to Update, you haven't fixed the cross-threading problem.  This line of code:

     

    Code Snippet

    rowArray = m_NewRows.ToArray();

     

    is creating a new array, but the DataRow objects it contains are the same ones that were in the old one.

     

    I suggested that you use the DataRows in the List to populate a new DataTable, and then pass that DataTable to the Update method.  You'll populate the DataRows' ItemArray with the ItemArray from the original DataRow.  Yes, this means that you'll still be using the same objects across threads.  But access to the objects in the specific columns is genuinely read-only, so that shouldn't cause a problem.

     

    I don't know that this is what's causing the problem in the UI that you're observing, but it could be.

     

    Also, your strongly-typed DataTable should in fact contain a method that lets you specify all of the columns for a row when you add it.  There should be two overloads of the Add[TableName]Row method.  One of them takes a [TableName]Row object, and the other's argument list should map onto the table's mutable fields.

    Wednesday, March 19, 2008 7:06 PM
  • Yes, but the .AddMyCustomRow(field1, field2, ...) function does not *return* the DataRow that was just add

    and you're right, I'm still suffering from cross-thread operations.  I will try the approach of rebuilding a new DataTable.

    One other thought -- rather than keeping track of the added/changed rows, can't I simply call the .GetChanges() (which returns a copy of the DataTable rows that have changed, and then call AcceptChanges on the original table, and finally call Update on the changes table? 


    e.g.

    Code Snippet

    lock (m_DataTable) {
        changes = m_DataTable.GetChanges();//.Copy();
        m_DataTable.AcceptChanges();
    }
    if (changes != null) {
        m_TableAdapter.Update((MyCustomDataTable)changes);
    }


    Wednesday, March 19, 2008 7:44 PM
  • The constructor for the DataRow is hidden because the DataRow always belongs to one and only one DataTable.  They could have made a constructor that with the DataTable as a required parameters, but a factory method on the DataTable is a more sensible idiom.

     

    Using GetChanges seems like the obvious thing to do; I've just never used it, so it didn't occur to me.  Did it work?

    Monday, March 24, 2008 7:13 PM
  • It did work, and from preliminary speed tests it seems to be performing nicely -- I lock only long enough to make the call to GetChanges.

    Thanks again for all your help.

    Rick
    Tuesday, March 25, 2008 12:08 PM