1

I have a combo box in main UI where i will display some available items list. I have one background worker thread which will run time check for available items.

I am running this thread in loop so that for each time it will check items available and update the to main UI combobox.

This is my code snippet.

do
{
    iNumberofReaders = 0;

    this.Dispatcher.BeginInvoke((Action)(() =>
    {
        NumberOfCards.Items.Clear();
    }));

    //
    // Compose a list of the card readers which are connected to the
    // system and which will be monitored.
    //
    ArrayList availableReaders = this.ListReaders();

    this._states = new ReaderState[availableReaders.Count];

    for (int i = 0; i <= availableReaders.Count - 1; i++)
    {
        this._states[i].Reader = availableReaders[i].ToString();
    }

    result = (SmartcardErrorCode)UnsafeNativeMethods.GetStatusChange(
                this._context, 1000, this._states, this._states.Length);

    szAvailableReaders = new string[availableReaders.Count];
    for (int i = 0; i < availableReaders.Count; i++)
    {
        if (0 != this._states[i]._attribute)
        {
            szAvailableReaders[iNumberofReaders] = this._states[i]._reader;
            iNumberofReaders++;
        } // if
    } // for

    if (iNumberofReaders > 1)
    {
        this.Dispatcher.BeginInvoke((Action)(() =>
        {
            for (int j = 0; j < iNumberofReaders; j++)
            {
                NumberOfCards.Visibility =system.Windows.Visibility.Visible;
                NumberOfCards.Items.Add(szAvailableReaders[j]);
            } // for
        }));
    }
} while (SetThread);

But this code in 2nd iteration throwing

Object reference not set to an instance of an object

An unhandled exception of type 'System.NullReferenceException' occurred

If I comment combobox add element then its working fine.

constant string in add element also throwing same error. so i beleive problem is with something in for loop and dispatcher begininvoke.

NumberOfCards.Items.Add("ABCD");

(NumberOfCards is ComboBox)

I have observed strange behaviour in for loop in dispatcher begin invoke. Can anyone help?

Nimish
  • 709
  • 3
  • 16
  • possible duplicate of [What is a NullReferenceException and how do I fix it?](http://stackoverflow.com/questions/4660142/what-is-a-nullreferenceexception-and-how-do-i-fix-it) – EKrueger Jun 26 '14 at 11:09
  • 1
    On which line is your exception thrown? Is it `NumberOfCards.Items.Add(szAvailableReaders[j]);`? I'm assuming `NumberOfCards` is your combobox... – Dan Puzey Jun 26 '14 at 11:13
  • yes is believe it is throwing exception in NumberOfCards.Items.Add(szAvailableReaders[j]); I think dispatcher begin invoke is running contineously despite of for loop – Nimish Jun 26 '14 at 11:14
  • messagebox in dispatcher begin invoke showing only one value. its not properly looping – Nimish Jun 26 '14 at 11:19

1 Answers1

0

You are modifying the szAvailableReaders array in a separate thread, while reading it in your GUI thread.

First, you should consider using Invoke instead of BeginInvoke, as the latter queues requests on a GUI thread and can easily create a bunch of queued actions which will unnecessarily hog the CPU.

Additionally, the simplest thing to do when sharing data between threads is to avoid sharing the same instance, and instead create a immutable copies of your data for the UI thread. That also goes for iNumberofReaders and any other fields you are sharing between threads.

In other words, the background loop should look something like:

do
{
    var availableReaders = this.ListReaders();

    // If you're only using `states` in this method,
    // make it a local variable:
    var states = availableReaders
        .Select(r => new ReaderState() { Reader = r.ToString() })
        .ToArray();

    // fill the data
    result = (SmartcardErrorCode)UnsafeNativeMethods
        .GetStatusChange(this._context, 1000, states, states.Length);

    // this is the important part: create a new instance of reader names.
    // LINQ also simplifies it a bit:        
    var availableReaders = states
        .Where(s => s._attribute != 0)
        .Select(s => s._reader)
        .ToList();

    // now that you have a local list, you can simply swap the field 
    // which is referencing it (since object assignments are atomic in .NET).
    // So the field gets a unique copy every time, and you can even go
    // an extra step forward by declaring it as a `ReadOnlyCollection<string>`:
    szAvailableReaders = new ReadOnlyCollection<string>(availableReaders);

    // Consider using `Invoke` here:        
    this.Dispatcher.BeginInvoke((Action)(() =>
    {
        NumberOfCards.Items.Clear();
        NumberOfCards.Items.AddRange(szAvailableReaders.ToArray());
    }));

} while (SetThread);

Apart from that, this looks like a really tight loop. I am not sure what you're reading here, but shouldn't you use some kind of a "readers list changed` event and only update the combo in that case? Or at least use a timer and poll the list every second or so?

vgru
  • 49,838
  • 16
  • 120
  • 201
  • groo here i am listing all connected smart card readers to system. GetStatusChange will give me actual connected readers. after that i am adding it to string array and if number of readers are more than 1 then only i am adding it into combo box.this loop will run continuously and check for connected readers. my goal here is to list readers in each loop.then clear combo box and add again connected readers. – Nimish Jun 26 '14 at 12:19
  • Yes, I got that. But I don't think your users want you app to use 100% of a single CPU core in order to update a combo box hundreds of times per second. – vgru Jun 26 '14 at 12:42
  • ya i know that. you have any suggestion on this or any way to make it work?? – Nimish Jun 26 '14 at 12:50
  • Doesn't the code here fix your `NullReferenceException`? – vgru Jun 26 '14 at 14:44
  • sorry i couldn't make it work. and i didn't get what you are doing there.(sorry i don't have any knowledge in c#). error is error CS1061: 'System.Collections.ArrayList' does not contain a definition for 'Select' and no extension method 'Select' accepting a first argument of type 'System.Collections.ArrayList' could be found (are you missing a using directive or an assembly reference?) – Nimish Jun 27 '14 at 03:52
  • i have kept a message box to check number of items before clear combobox and after clear combo box. each time it keeps increasing number of items. it is not properly clearing combobox items. that could be a problem???. I beleive problem is with clear combobox. i have used select index = -1 instead of clear. it is working but its duplicating the list. – Nimish Jun 27 '14 at 04:13
  • @Nimish: ok, so the `ListReaders` returns an `ArrayList`. You might want to make it return a type-safe generic list (e.g. `List`, or how is your class called anyway). Or you can call the `Cast` method in the line where you're calling it (`var availableReaders = this.ListReaders().Cast();`). I don't know how your readers class is called, but you should get the idea. Generally, you should try to avoid `ArrayList` and use generic lists instead (e.g. `List` and similar) for type safety and better compile-time hints. And the latter support LINQ queries. – vgru Jun 27 '14 at 13:12
  • ya groo i will try that – Nimish Jun 30 '14 at 04:24