9

We have a Grid that's bound to a List<T> of items. Whenever the user hits "Refresh", changes are obtained from the database, and the bound list is updated. I'm running into a problem where duplicate items are being added to the grid, and I cannot figure out why.

The database call returns two values: a List<int> of the Record Ids that have changed, and a List<MyClass> of updated data for the records that have changed. The existing code code I'm debugging which finds out what needs to be updated looks something like this:

public void FindUpdates(IList<MyClass> existingRecords,
                    IList<MyClass> updatedRecords,
                    List<int> updatedIds,
                    out IDictionary<int, int> existing,
                    out IDictionary<int, int> updated,
                    out List<int> updates,
                    out List<int> removes,
                    out List<int> adds)
{
    updates = new List<int>();
    removes = new List<int>();
    adds = new List<int>();

    existing = FindUpdated(existingRecords, updatedIds);
    updated = FindUpdated(updatedRecords, updatedIds);

    if (updatedIds != null)
    {
        // split add/update and remove
        foreach (int id in updatedIds)
        {
            if (!existing.ContainsKey(id))
                adds.Add(id);
            else if (updated.ContainsKey(id))
                updates.Add(id);
            else
                removes.Add(id);
        }

        WriteToLog(updatedIds, adds, updates, removes);
    }
}

private IDictionary<int, int> FindUpdated(IList<MyClass> records, List<int> updatedIds)
{
    IDictionary<int, int> foundItems = new Dictionary<int, int>();

    if (records != null && updatedIds != null)
    {
        for (int i = 0; i < records.Count; i++)
        {
            IMyClass r = records[i] as IMyClass ;
            if (r != null && !r.IsDisposed)
            {
                if (updatedIds.Contains(r.Id))
                {
                    foundItems.Add(r.Id, i);
                }
            }
        }
    }

    return foundItems;
}

The result of calling FindUpdates() is I get a Dictionary<Id, Data> of existing records, a Dictionary<Id, Data> of the updated records to replace them with, and a List<int> of Ids for which items should be Added, Removed, or Updated from the data source.

On occasion, a record gets added to the grid twice and I cannot for the life of me figure out where this code is going wrong.

I pulled the log file from one of these instances, and can clearly see the following sequence of events:

  • Item #2 added to the List of data
  • 20 minutes later, Item #2 is added to the List of data again

WriteToLog() from the 2nd add tells me that

  • updatedIds contains values 1, 2, and 3
  • adds contains 1 and 2
  • updates contains 3

Based on other log entries, I can clearly see that item #2 was added previously, and never removed, so it should be in the existingRecords variable have shown up in the updates variable, not in the adds. In addition, item #2 was successfully updated a few times between the first add and the second one, so the code in theory should work. I also have a screenshot of the UI as well showing both copies of item #2 in the grid of data.

Notes...

  • IsDisposed is only set to true in the overridden .Dispose() method on the item. I don't think this would have occurred.

    Edit: I've added a log statement since then, and can verify that IsDisposed is not set to true when this happens.

  • This has happened a few times now to a few different users, so it's not just a one-time thing. I am unable to reproduce the problem on-demand though.

  • The Grid of records can be fairly large, averaging a few thousand items.

  • I haven't ruled out the idea of the DB call returning invalid values, or lists which don't have the same items, however I don't see how that could affect the outcome

  • The one time I was able to see this bug in action, we were running some tests and other users were modifying record #2 fairly frequently

  • This all runs in a background thread

  • Based on the log, this was only running once at the time. It ran previously a minute before, and next 2 minutes later.

  • From the log file, I can see that item #2 has been updated a few times correctly before being incorrectly added a second time, so this code did work with the existing data set a few times before.

Is there anything at all in the code shown above that could cause this to happen? Or perhaps a rare known issue in C# where this could happen that I'm not aware of?

14
  • 2
    Are you asking if List.Contains() is thread-safe? If multiple threads are operating on shared state that exists outside of a mutex/semaphore/lock of some kind, I would spend my time there rather than blaming the data structure.
    – 48klocs
    Commented Mar 5, 2014 at 15:39
  • @Rachel: Sorry. Blind spell. :/
    – Jon
    Commented Mar 5, 2014 at 15:45
  • @Jon No problem, I edited the title anyways to hopefully clear up any confusion, and to better match what I really want to know :)
    – Rachel
    Commented Mar 5, 2014 at 15:53
  • @Rachel In your FindUpdated method this line: else if (updated.ContainsKey(id)) shouldn't be like this? (with negation (!) operator) else if (!updated.ContainsKey(id)) Commented Mar 5, 2014 at 16:23
  • 2
    It doesn't look wrong. It's not how I'd've written it - I'm not sure why FindUpdated doesn't return a Set<int> for example, or why you don't use a foreach - but it doesn't look algorithmically wrong at all. The thing that makes me worried is when you said it runs in a background thread. Does any other thread have code which could modify the objects in the out parameters? Since you mentioned threads, this kind of thing just stinks of a concurrent update bug. Although I'll admit the route by which this could happen looks highly implausible, since you primarily work with local data. Commented May 13, 2014 at 14:20

2 Answers 2

1

There is no reason an existing item would not be found in List<T> using the code above.

I should have had alarm bells in my head when I noticed the output values included Dictionary<int,int> of values containing the Id of an item and the index of the item within the existing list.

What was happening is items were being removed using existingRecords[existing[id]], where existing[id] would return the Index of the item within existingRecords. In order for this to work, items have to be removed from the largest index down. If a smaller index gets removed before a larger one, then the larger index is now incorrect by 1 position and the wrong item gets removed.

As to why I was getting duplicate items, that's because two collections were being maintained, one that was being used to determine if items were new/updated/deleted, and another that was being bound for display in the UI. The first collection was getting incorrectly updated, while the 2nd was not, resulting in scenarios where items could get added multiple times to the UI collection.

My short-term solution was to sort to the removes collection to ensure it was sorted by the index of each item in descending order. My long term solution will be to rewrite the code. :)

0
List.Contains(r) 

would try to find object a in the list that a.Equals(r). if MyClass doesn't override Equals you can't be certain that two different objects of the same class are equal. I'm not completely sure about this: but I think equals uses GetHashCode(), If you decide to override one of them, you're supposed to override the other.

here's MSDN about it: http://msdn.microsoft.com/en-us/library/bhkz42b3(v=vs.110).aspx

3
  • 1
    It's a list of integers.
    – LarsTech
    Commented Mar 5, 2014 at 16:32
  • 1
    It's correct that .Contains() compares objects by reference, however in my case I have a List<int>, which should correctly compare integers by their value.
    – Rachel
    Commented Mar 5, 2014 at 16:42
  • I don't understand why this is getting upvotes. I am not using List.Contains(r), I am using List.Contains(r.Id), and Id is a generic int which always evaluates the same way.
    – Rachel
    Commented May 13, 2014 at 15:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.