Question Problem with Listbox

[digitaldrew]

Trying to write a small program that will take each website in a listbox and ping them.

Everything seams to be working fine, however it seams like the loop goes so fast the txtLog doesn't have time to update itself. I've gone in and added a thread.sleep to put a pause in there but it doesn't seam to fix the issue. The program will pause, but the txtLog still doesn't update. However, if I stick a messagebox in the code then it will give me the messagebox that I requested, and then update the text log appropriately before continuing to the next website..

Here is the start button code. Basically, they click Start and it finds out which radio button they have selected so it knows how many times to ping each address. Then it starts the loop where it should check each item..

    Private Sub btnStartPing_Click(sender As System.Object, e As System.EventArgs) Handles btnStartPing.Click

        If lstIps.Items.Count <= 0 Then
            MsgBox("Please Add at Least One IP or Website!")
            Exit Sub
        ElseIf chkOnePing.Checked = True Then
            pingNumber = 1
        ElseIf chkFourPings.Checked = True Then
            pingNumber = 4
        ElseIf chkCustom.Checked = True Then
            Try
                Integer.Parse(txtCustomPing.Text)
                pingNumber = Me.txtCustomPing.Text
            Catch ex As Exception
                MessageBox.Show("Custom Value Must be a Number!")
            End Try
        Else
            MsgBox("Please Select How Many Pings to Send!")
            Exit Sub
        End If

        Do
            For i As Integer = 0 To lstIps.Items.Count - 1
                lstIps.SelectedIndex = i
                selectedPing = lstIps.SelectedItem.ToString()
                Call pingIt()
                'Thread.Sleep(2000)
            Next
            pingNumber = pingNumber - 1
        Loop Until pingNumber <= 0
        MsgBox("Pinging Completed!")

    End Sub

When it calls the pingIt sub that is where it's actually performing the ping..Here is the code for that

    Public Sub pingIt()
        Dim pingSender As New Ping()
        Dim options As New PingOptions()
        Dim roundtriptime As String

        ' Use the default Ttl value which is 128,
        ' but change the fragmentation behavior.
        options.DontFragment = True

        ' Create a buffer of 32 bytes of data to be transmitted.
        Dim data As String = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
        Dim buffer() As Byte = Encoding.ASCII.GetBytes(data)
        Dim timeout As Integer = 120

        'MsgBox(selectedPing)
        Dim reply As PingReply = pingSender.Send(selectedPing, timeout, buffer, options)
        lblTotalPingsSent.Text = lblTotalPingsSent.Text + 1

        If reply.Status = IPStatus.Success Then
            MsgBox("Success")
            roundtriptime = GetMs(reply.RoundtripTime)
            txtLog.AppendText("Ping Successful to: " & selectedPing & " with Roundtrip Time: " & roundtriptime & " at: " & TimeOfDay & vbCrLf)
            lblTotalPingsReceived.Text = lblTotalPingsReceived.Text + 1
            Exit Sub
        Else
            txtLog.AppendText("Ping UnSuccessful to: " & selectedPing & " at: " & Now & vbCrLf)
            lblTotalPingsLost.Text = lblTotalPingsLost.Text + 1
            Exit Sub
        End If
    End Sub

As you see, I have the MsgBox(selectedPing) commented out in there. If I uncomment that then I'll get the correct msgbox with the string and everything after that will perform just fine. However, if I uncomment the msgbox then everything still goes as it should - but the txtLog doesn't update.

Any idea why this happening? What can I do so that it actually updates appropriately?

 

[IanRyder]

Hi,

It's doing this because you are still doing it wrong and not understanding what the second thread is doing. To try and help you get your head round this, consider this:-

Imaging if you will, two processes that start together:-
Process 1 runs every 1 millisecond (this is your Do Loop and your For Loop). Due to the loops, this process will run, for lets say, 30 iterations and set a variable called selectedPing to an IP address on each iteration of the loop. This process therefore completes in 30 milliseconds.

Process 2 runs and executes every 1 second due to the work it has to do but it also uses a variable called selectedPing (which happens to be a privately declared variable at the class level) to display the work that was done. The problem is the selectedPing in now equal to the last IP address in the loop in process 1 whereas process 2 is still working on trying to display the result of the FIRST ping in process 1.

Does that make sense to describe what is basically happening in the two threads?

To the fix then:-

1) In the pingSender.SendAsync(selectedPing, timeout, buffer, options, waiter) call change the userToken parameter to selectedPing so that this variable can be passed to the PingCompletedCallback routine. i.e:-

pingSender.SendAsync(selectedPing, timeout, buffer, options, selectedPing)

2) In the PingCompletedCallback routine add this line of code after the declaration of your Reply variable:-

Dim myPingedIPAddress As String = CType(e.UserState, String)

UserState contains the value of the userToken in the pingSender.SendAsync method

3) Charge the declaration of DisplayReply(reply) to:-

DisplayReply(reply, myPingedIPAddress)

4) Update the DisplayReply routine to something like:-

Public Sub DisplayReply(ByVal reply As PingReply, ByVal myPingedIPAddress As String)
  If reply.Status = IPStatus.Success Then
    txtLog.AppendText("Pinged: " & myPingedIPAddress & " with Successful Reply From: " & reply.Address.ToString & " with Roundtrip Time: " & reply.RoundtripTime & " and TTL: " & reply.Options.Ttl & vbCrLf)
  Else
    txtLog.AppendText("Ping Failed to : " & myPingedIPAddress & " Status : " & reply.Status.ToString & vbCrLf)
  End If
End Sub

Notice, not a single selectedPing variable in sight.

Hope that helps.

Cheers,

Ian

 

[digitaldrew]

Hey Ian..Thanks for taking a second to lay it out there like that for a newbie like me! I understand what you're saying and what is happening now. The selectedPing is what's throwing things off because two seperate threads are accessing it and basically reading /displaying different things because one can't keep up with the other.

I've made the changes you suggested and now things seam to be working much smoother now aside from two small issues. So, basically instead of continuing to use the same string (SelectedPing) in multiple places you took it and set it as the usertoken and then converted it back over to a string as myPingedIPAddress - and then proceeded to call it from there in DisplayReply.

As for the issues now..First, the log doesn't update after each ping..it updates appropriately now with all domains showing up - but the log isn't updated until after all pings are finished. Secondly, if Ping.Async is running in it's own thread then other features of the program should be accessible and clickable, correct? When the pinger is running the Stop button and everything else is still unclickable/unaccessible.

Thanks again!

 

[IanRyder]

Hi,

Don't forget that the code to Send the ping's is still on the UI thread, so while you are looping to send all the Ping's then you will still be tying up the UI thread while that loop is running. What is on the second thread at the moment is the physical act of pinging your sites.

This is where a BackgroundWorker would come in handy. You could pass the list of IP addresses to be pinged to the BackgroundWorker so the BackgroundWorker does all the looping in a second thread and then your call to pingSender.SendAsync does the actual pinging in a third thread. This would ensure that the initial UI thread stays responsive at all times.

Have a look here:-

BackgroundWorker Class (System.ComponentModel)

Hope that helps.

Cheers,

Ian

 

[digitaldrew]

Hey Ian,
Thanks for your reply. I've been looking into the background worker and have been working to get this implemented into it..Only a couple questions though:

1) The background worker has it's own ProgressChanged and RunWorkerCompleted events..How would I go about using those along with the PingCompletedCallback event? Since the PingCompletedCallback is looking for the reply?

2) When I do a reportprogress from inside the dowork sub, what kind of string would I be looking to pass to it *example: reportprogress(0,STRING)*? the usertoken? Since the PingCompletedCallback is taking the reply and passing it off to DisplayResult I can't see what would need to be passed to the ReportProgress..

3) The DisplayReply sub is where I'm actually looking at to see if the reply was successful..then, depending on if the reply is a success or not -im posting it into the txtLog. By using the background worker will I run into any cross-threading issues by using the PingCompletedCallback which calls then DisplayReply which in return tries to post a message to the txtLog?

I hope I'm making some sense here..I've been having problems dealing with PingCompletedCallback/DisplayReply and ReportProgress/RunWorkerCompleted as it seams like they are doing the same thing..

 

[IanRyder]

Hi,

In this case, but only for the moment, I am not going to post an example this time since I feel you have got everything that you need to get this working. The only thing you maybe missing, once again, is how the multi-threading will work with what you are doing.

Obviously, you already know that you cannot cross-thread, so think it through logically. If you start on Thread1 and pass something to Thread2 which in turn passes something else to Thread3, then Thread3 needs to pass something BACK to Thread2 which in turn needs to pass something BACK to Thread1 for you to update the UI controls on the form.

Hope that helps and I will provide an example for you if you still cannot get your head round it.

Cheers,

Ian

 

[digitaldrew]

Thanks for your response Ian. After reading your message I am now fairly certain the PingCompletedCallback and DisplayResult are still both going to come into play here. We know that the cross-threading error is going to occur by leaving the DisplayResult inside PingCompletedCallback, so I am thinking that will need to be getting called from inside the reportedprogress? This is really the only place I could see that going..

I would like to try and get this figured out without you taking up your whole evening writing examples..But just a few other questions:
1) Am I going to be using the worker.ReportProgress in my DoWork sub?
2) Should I be looking to call the DisplayResult from inside the ProgressChanged sub?

Thanks again

 

[IanRyder]

Hi,

You have got it now. To be specific:-

Am I going to be using the worker.ReportProgress in my DoWork sub?

Yes, you then code the BackgroundWorker.ProgressChanged event to show the results being passed by worker.ReportProgress.

Should I be looking to call the DisplayResult from inside the ProgressChanged sub?

Yes

Good luck.

Cheers,

Ian

 

[IanRyder]

Hi,

Actually, after thinking that through a little, that's not quite right.

You will call the sendPing.Async from the DoWork event of the background worker, but then the results will be reported back to your PingCompletedCallback routine.

So it will be in the PingCompletedCallback routine that you will need to do the worker.ReportProgress to update the UI thread.

This implies that you therefore need to declare your BackgroundWorker variable at the class level so that it is accessible from the PingCompletedCallback routine.

That should be about it.

Cheers,

Ian

 

[digitaldrew]

Hey Ian..thanks for responding and updating that. For the life of me I couldn't think of what I could pass through reportprogress inside the dowork sub (other than the selectedPing which would have been useless lol)...Now that you let me know that I was able to figure it all out..got it working nice and smoothly now with..

    Private Sub pingThread_DoWork(sender As System.Object, e As System.ComponentModel.DoWorkEventArgs) Handles pingThread.DoWork
        Dim worker As BackgroundWorker = CType(sender, BackgroundWorker)
        Do Until pingNumber <= 0
            For x As Integer = 0 To lstIps.Items.Count - 1
                If lstIps.InvokeRequired Then
                    lstIps.Invoke(Sub() lstIps.SelectedIndex = x)
                End If
                If (worker.CancellationPending = True) Then
                    e.Cancel = True
                    Exit For
                Else
                    selectedPing = lstIps.Items(x).ToString

                    Dim pingSender As New Ping()

                    AddHandler pingSender.PingCompleted, AddressOf PingCompletedCallback
                    Dim data As String = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
                    Dim buffer() As Byte = Encoding.ASCII.GetBytes(data)
                    Dim timeout As Integer = 2000
                    Dim options As New PingOptions(64, True)
                    pingSender.SendAsync(selectedPing, timeout, buffer, options, selectedPing)
                    Thread.Sleep(2000)
                End If
            Next
            pingNumber = pingNumber - 1
        Loop
    End Sub

and

    Private Sub PingCompletedCallback(ByVal sender As Object, ByVal e As PingCompletedEventArgs)
        ' If the operation was canceled, display a message to the user.
        If e.Cancelled Then
            txtLog.AppendText("Ping canceled at: " & TimeOfDay & vbCrLf)
            MsgBox("Pinger Successfully Cancelled!")
        End If

        ' If an error occurred, display the exception to the user.
        If e.Error IsNot Nothing Then
            txtLog.AppendText("Ping failed:" & e.Error.ToString() & vbCrLf)
        End If

        Dim reply As PingReply = e.Reply
        pingThread.ReportProgress(0, reply)

    End Sub

and

    Private Sub pingThread_ProgressChanged(ByVal sender As Object, ByVal e As ProgressChangedEventArgs)
        DisplayReply(e.UserState, selectedPing)
        ProgressBar1.PerformStep()
    End Sub

I really appreciate all your help and time on this one. I'm a newbie and you're little lesson here on threads has helped me tremendously! I'm sure if you look through all the code above you will see other mistakes..If you see anything that looks like it could use tidying up please let me know!

 

[IanRyder]

Hi,

Good to hear that you have got it working the way you want and glad I could help.

I have not had a proper look at the code but just a couple of quick ones for you. You are using that selectedPing variable in your Reply again so I get the feeling that is wrong and the handles clause is missing from your ProgressChanged event but I assume that is just a typo?

Cheers,

Ian

 

[JohnH]

You are still sleeping the thread that starts the ping for 2 seconds, the same time you have set as ping timeout. If your intension, like it is now, is to send the pings synchronously you are way overcomplicating and should go back to using the Ping.Send method. The difference now is that you're using the BackgroundWorker component and can report back to UI thread using its functionality for that.

 

[digitaldrew]

@JohnJ..What I was hoping to do was have it select the first website in the listbox, ping it, report the status, pause for a couple seconds, then move to the next item in the list and perform the same thing. When using the thread.sleep things seam to work nice and smoothly. However, once the thread.sleep is removed I find that it speeds through the entire list too quickly (before it can report the progress)..Am I missing something here? If the thread.sleep is set for 2seconds should the timeout be for something smaller?

@Ian..Thanks for noticing that. It was getting late and I had been doing so much cutting/pasting I forgot to put that back in. I think this should look a little better..

Public myPingedIPAddress As String = String.Empty

    Private Sub PingCompletedCallback(ByVal sender As Object, ByVal e As PingCompletedEventArgs)
        ' If the operation was canceled, display a message to the user.
        If e.Cancelled Then
            txtLog.AppendText("Ping canceled at: " & TimeOfDay & vbCrLf)
            MsgBox("Pinger Successfully Cancelled!")
        End If

        ' If an error occurred, display the exception to the user.
        If e.Error IsNot Nothing Then
            txtLog.AppendText("Ping failed:" & e.Error.ToString() & vbCrLf)
        End If

        Dim reply As PingReply = e.Reply
        myPingedIPAddress = CType(e.UserState, String)
        pingThread.ReportProgress(0, reply)

    End Sub

and

    Private Sub pingThread_ProgressChanged(ByVal sender As Object, ByVal e As ProgressChangedEventArgs)
        DisplayReply(e.UserState, myPingedIPAddress)
        ProgressBar1.PerformStep()
    End Sub

 

[JohnH]

@JohnJ..What I was hoping to do was have it select the first website in the listbox, ping it, report the status, pause for a couple seconds, then move to the next item in the list and perform the same thing. When using the thread.sleep things seam to work nice and smoothly. However, once the thread.sleep is removed I find that it speeds through the entire list too quickly (before it can report the progress)..Am I missing something here? If the thread.sleep is set for 2seconds should the timeout be for something smaller?

You still don't need Ping.SendAsync since you only actually perform pings synchronously.

 

[digitaldrew]

Thanks John..You are correct. Removing the ping.async and then removing the thread.sleep seamed to make things work much smoother. I was also able to remove the pingcompletedcallback all together and send the pingreply straight over to progresschanged! I dropped the timeout speed and it seams to go through the list at a nice rate now..slightly faster than I had the thread.sleep set for but better!