views:

29

answers:

1

hi. In a datagridview I have an IP address field. when I click on check status button I make thread for each row in datagridview and then call a remote object on the host on that IP and get some information and set another datagridview field as thath info.

but there is a problem. the info is wrongly set on datagridview. why?

    private void button_CheckStatus_Click(object sender, EventArgs e)
    {
        for (int i = 0; i < dataGridView_VSD.Rows.Count; i++)
        {
            IPAddress IP;
            if (IsValidIP(dataGridView_VSD["VSD_IP", i].Value.ToString(), out IP))
            {
                Thread t = new Thread(() => CheckVSDStatusThreadFunction(IP, i));
                t.Start();
            }
        }

    }

private bool IsValidIP(string t, out IPAddress IP)
{
    try
    {
        IPAddress ip = IPAddress.Parse(t);
        IP = ip;
        return true;
    }
    catch
    {
        IP = null;
        return false;
    }
}


private void CheckVSDStatusThreadFunction(object IP, object rowIndex)
{
    IRemoteUpdate temp = (IRemoteUpdate)Activator.GetObject(typeof(IRemoteUpdate), "tcp://" + ((IPAddress)(IP)).ToString() + ":22000/BRU");
    try
    {
        string s = temp.GetInfo();
        SetDataGridViewText(s,"VSD_Info", (int)rowIndex);
    }
    catch
    {
        SetDataGridViewText("can not estalish connection to VSD", "VSD_UpdateStatus", (int)rowIndex);
    }
}


delegate void delSetDataGridViewText(string text, string columnname,int rowindex);
private void SetDataGridViewText(string text,string columnname, int rowindex)
{
    if (dataGridView_VSD.InvokeRequired)
    {
        delSetDataGridViewText d = new delSetDataGridViewText(SetDataGridViewText);
        this.Invoke(d, new object[] { text, columnname, rowindex });
    }
    else
    {
        dataGridView_VSD[columnname, rowindex].Value = text;
    }
}
+1  A: 

Make sure not to capture the loop variable:

    for (int i = 0; i < dataGridView_VSD.Rows.Count; i++) 
    { 
        int ii = i;
        IPAddress IP; 
        if (IsValidIP(dataGridView_VSD["VSD_IP", i].Value.ToString(), out IP)) 
        { 
            Thread t = new Thread(() => CheckVSDStatusThreadFunction(IP, ii)); 
            t.Start(); 
        } 
    } 

This is a very common mistake.

See e.g. here

Henrik
I used this solution but why it should be correct???
HPT
@HPT: added a link to my answer.
Henrik
@HPT, see [this article](http://blogs.msdn.com/b/ericlippert/archive/2009/11/12/closing-over-the-loop-variable-considered-harmful.aspx) for an explanation. All your anonymous delegates are capturing the same variable `i`, so it has the same value for all of them.
Thomas Levesque