best way to go about this

[Uranium-235]

so, i'm making a small text chat program that uses a WebClient and xml parsing on the returning text (PHP scripts from SSL/HTTPS server). First my 'heartbeat' was a timer, but that made the gui kinda freeze for fractions of seconds when I was dragging the box around

I decided to make a backgroundworker, gui was smooth but I can't put anything in the message box cause of this cross thread problem, which i've done some research on and people suggested it not be used (CheckForIllegalCrossThreadCalls). I used it and it worked just fine. What if I'm just adding text to a richtextbox, That textbox is only hit by this one background process, so i'm not sure how the main thread and worker thread could collide.

I'm also trying to 'collect' messages in a dictionary or collection and make a timer execute and show the messages in the collection and reset the collection at the end of the bgworker call and stop the timer at the end of the tick, since timers are non-blocking, but its not executing, can you start a timer from a background worker?

anyone have any better solutions, please go ahead. All I need to do is hammer through these small problems and the rest of it should be simple

[Evil_Giraffe]

No, don't set CheckForIllegalCrossThreadCalls to false. Look at the name. "Check for Illegal Cross Thread Calls". And you're asking it to not check for those Illegal calls. It isn't called "DontMagicallyMakeIllegalCrossThreadCallsLegal". Those calls are still illegal and can still absolutely corrupt your program state. The fact that it seems to "work just fine" is just luck and in large part that you're running this with the IDE's debugger attached and compiled into Debug mode where the timings are so vastly different to running it "for real" that you will probably never see the pain you'll inflict on your users with your app randomly crashing.

The correct thing to do is to Invoke onto the UI control's owning thread. To do this, you need to put all the updates you want to make to that control into a separate method, then that method needs to firstly check whether your control needs invoking upon with the InvokeRequired property, if this is true then you need to Invoke your method again, otherwise you are able to update the control. It looks something like this:

Imports System.Threading
 
Public Class Form1
 
    Private Sub Form1_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        Dim backgroundThread As New Thread(AddressOf BackgroundProcessing)
        backgroundThread.Start()
    End Sub
 
    Private Sub BackgroundProcessing()
        UpdateLabel("This was from a background thread")
    End Sub
 
    Private Sub UpdateLabel(message As String)
        If Label1.InvokeRequired Then
            Label1.Invoke(New Action(Of String)(AddressOf UpdateLabel), message)
        Else
            Label1.Text = message
        End If
    End Sub
End Class

However, if you are running all your network processing in a loop inside a BackgroundWorker, you have the option not to worry about this. The BackgroundWorker has a ProgressChanged event that it raises on the UI thread. From the DoWork method you can call ReportProgress and supply a UserState object that could be your message. Like this:

Public Class Form2
 
    Private Sub Form2_Load(sender As Object, e As EventArgs) Handles MyBase.Load
        BackgroundWorker1.WorkerReportsProgress = True
        BackgroundWorker1.RunWorkerAsync()
    End Sub
 
    Private Sub BackgroundWorker1_DoWork(sender As Object, e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
        BackgroundWorker1.ReportProgress(0, "This came from the background worker")
    End Sub
 
    Private Sub BackgroundWorker1_ProgressChanged(sender As Object, e As System.ComponentModel.ProgressChangedEventArgs) Handles BackgroundWorker1.ProgressChanged
        Dim message As String = DirectCast(e.UserState, String)
 
        Label1.Text = message
    End Sub
End Class

However, I'm not too keen on this approach myself because you're not actually reporting progress, so the use of a BGW seems somewhat suspect (an infinite loop within a BGW is a definite smell in my book).

[wakawaka]

I don't know how people feel about this, but you can make a seperate class that has an event that gets raised when a HeartBeat occurs. This way the UI can subscribe to the event and you can code the event to check the invocation list and attempt to cast the target as a ISynchronizeInvoke object.

Public HeartBeatOccured As EventHandler(Of EventArgs)

'....
Public Overridable Sub OnHeartBeatOccured(e As EventArgs)
	Dim heartBeatOccured = HeartBeatOccured


	If heartBeatOccured IsNot Nothing Then
		For Each d As Delegate In ProgressUpdate.GetInvocationList()
			'Gettings the sync context from the target and then raising the event
			Dim syncContext = TryCast(d.Target, System.ComponentModel.ISynchronizeInvoke)

			'if the SynchronizationContext isn't null, it is most likely a UI or the user assigned on so we will raise the event on that thread
			If syncContext IsNot Nothing Then

				'Call the target method in a Async fashion
				syncContext.BeginInvoke(d, New Object() {Me, e})
			Else
				'This is called is there is no synchronization context assigned to the target so invoke it on the calling thread
				d.DynamicInvoke(New Object() {Me, e})
			End If
		Next
	End If
End Sub
'...

I do believe the ISynchonizeInvoke interface is really only applied to Form objects, but I would have to dig around to be certain of that.

[Shaggy Hiker]

I've used a different technique that is somewhat similar. I saved the SynchronizationContext for the UI in a class level variable prior to starting the thread. When I want to raise an event, I use .Post against the SynchronizationContext to call a method that raises the event. The method is run on the UI thread (the context), so the events are raised on the UI thread.

[Uranium-235]

why would MS even give the option to disable this if it causes so many problems?

anyways, progress monitoring is kind out of the question due to how I have it setup, or at least I think it is. does progressmonitoring change depending on where how far executed the whole background worker is? cause the background worker is simply a leap off for a single function, that has a one second thread sleeper at the bottom, and calls the same function again

but last night I came up with a possible idea. scratch the function, put it all in the background worker, and call runworkercompleted to pick up the messeges stored at the end of the worker, and at the end of that sub, call the runworkerasync to refetch

if that doesn't work your guys suggestions are my only hope, even though I don't entirely understand them

[Shaggy Hiker]

I'm curious as to why MS gave the option, as well. I suspect that there are some cases where it doesn't cause any problems, but you may have to know a whole lot more about the internal workings to know when it is safe to do so. Most cross-thread calls are left entirely to your discretion. Do it right and all is well. Do it wrong and you get the most difficult of all possible bugs to track down: Race conditions. Do it REALLY wrong and you can get a deadlock, which will freeze up your app solid, but which is generally much easier to diagnose. So, I can only surmise that it is so hard for most people to get cross thread operations correct when dealing with controls that MS just decided to forbid it....yet leave one little loophole for the wise, the lucky, and the foolish but brave.

So, the original design had the BGW calling a single function repeatedly, and the function (or the BGW) included a thread.sleep to pause it for a second? I can see uses for that, but what progressmonitoring are you talking about? If you are talking about the event that gets raised by the BGW, that is something you do yourself whenever you want to, and with whatever progress (or any other thing) that you want to report. Therefore, if I understand your design, I'd be inclined to go with the way you had it. It sounds like the background process might as well keep right on running for the duration, and as long as you are periodically sleeping the thread, the performance should be pretty good. If you don't sleep the BGW, you run the risk of spending too much CPU time whirring through that background process faster than it has any reason to operate.

As for the messages, an object at form scope is visible to the UI thread and to any threads spawned from the UI thread. Therefore, it will also be visible to the BGW. You mentioned a dictionary, but when it comes to messages, you should also take a look at both the Queue and the Stack, though the Queue is more likely to be suitable.

[Uranium-235]

the orignal design had a timer on a 1s interval. Calling it without a time interval, i'm more concerned about how much bandwidth and db access would take up on the server once you start to pool more than one of these programs

actually all I do is call the bgworker once and then re-interate the function. As you can tell this is more than just a chat program, but chat is the final piece of it, its actually a launcher for vnc software, with the technician software on my end monitoring the queue table

 Private Sub BackgroundWorker_DoWork(ByVal sender As System.Object, ByVal e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker.DoWork
        upd()
    End Sub

    Private Sub upd()
        'Try
        HeartBeat.Text = "♥"
        TickPostData.Clear()
        If Not TechID = -1 Then
            TickPostData.Add("connected", "true")
        End If

        TickPostData.Add("action", "tick")
        TickPostData.Add("session", SessionID)
        Dim VNCData As XmlReader = VNCTicker.PostValuesXML(TickPostData)


        VNCData.ReadToFollowing("response")
        VNCData.MoveToFirstAttribute()
        Dim action = VNCData.Value

        If action = "stats" Then
            VNCData.ReadToFollowing("numonline")
            Dim QueueSize = VNCData.ReadElementContentAsInt()
            VNCData.ReadToFollowing("placeinline")
            Dim InPlace = VNCData.ReadElementContentAsInt()
            ClientStatus.Text = "In Queue: " & InPlace & "/" & QueueSize
            'SystemMessage("System> Testing")
            Messages.Add(0, "test")
        ElseIf action = "connect" Then
            VNCData.ReadToFollowing("connectID")
            TechID = VNCData.ReadElementContentAsInt()
            VNCData.ReadToFollowing("techname")
            Dim TechName = VNCData.ReadElementContentAsString()

        ElseIf action = "connected" Then

        End If

        If Me.Visible = False Then
            Dim wc = New VNCSession
            wc.Post("action=deletefromqueue&session=" & SessionID)
            wc.close()
            BackgroundWorker.CancelAsync()
            BackgroundWorker.Dispose()
        Else
            HeartBeat.Text = ""
           
            Threading.Thread.Sleep(1000)
            upd()
        End If
        'Catch
        '    'we do nothing
        'End Try

    End Sub

this actually has it with my expirmental 'messages' dictionary, something i'm going to have to do with namevalue collections or something like that.

the problem was calling systemmessage(text), it supposed to change the text color on the rtftextbox and that dosen't crossthread, though updating actual text does without a problem, at least on my statusbar

the bottom of it with me.visible happened out of the problem I can't shut this thing down as the only form, if I try about 50% of the time I get a "Value dispose() cannot be called while doing CreateHandle()". so I keep the parent form open now. Whats wierder is if even if I close and dispose the chat from (even from the parent handle) the background worker keeps going, even if I call dispose to the background worker from the parent form. Which is why I did this, I can cancel async from within the worker and dispose it there it works fine

I have a nothing on my try/catch (currently commented out) on my entire function cause closing it in the middle of a work can sometimes cause the xml parser to act up

[Shaggy Hiker]

I'd be inclined to worry about the bandwidth, too. This thing looks like it run flat out unless you put something like that sleep in there. That would probably be more information than you could want.

This is going to cause you terrible trouble:

 HeartBeat.Text = ""
           
 Threading.Thread.Sleep(1000)
 upd()

The problem is that you are recursing. Each call to udp never ends, but calls udp again (unless you take the other branch and terminate the BGW itself). If you were to put a breakpoint on that Sleep call and watch the stack, you would find that you keep hitting the breakpoint...and the stack would keep getting bigger and bigger and bigger. Eventually, the program would crash when the stack ran out of memory. This would probably happen very fast if you removed the sleep, but the sleep statement probably saves you because you can likely run for hours before it crashes.

I would say that what you should be doing in the DoWork method looks more like this:

 Do While CancelPending = False 'Or something like that, I didn't look it up
  upd()
  Sleep(1000)
  ReportStatus 'I REALLY didn't look THAT one up, and it is wrong.
 Loop

I would then strip out the stuff from upd that deals with sleep, recursive calls to udp, anything dealing with any control on the form (I really like the heartbeat, but you would do that in the reportprogress event handler that you would be raising once you look up whatever I meant to write when I wrote ReportStatus), and quite possibly all the stuff in the If Me.Visible = False statement. Whatever that is, I think that belongs in the loop in DoWork, and not in the udp method.

That's my suggestion.

[Uranium-235]

I got rid of the function, put it all in the background worker

recalled async in the the backgroundworker complete function, I get "Exception has been thrown by the target of an invocation."

using try/catch in the complete function fixes this, but i'm curious what is causing this?

[wakawaka]

Do you have an EndInvoke to handle the async operation completing?

[Joacim Andersson]

why would MS even give the option to disable this if it causes so many problems?

Because Microsoft made a mistake in .Net 1.1 which allowed cross thread calls (and because of that most multi-threaded applications unexpectedly crashed), that was corrected in .Net 2.0 and they added this property to allow backward compatibility with .Net 1.1, however you should never use it.

[Uranium-235]

Do you have an EndInvoke to handle the async operation completing?

no I have just the worker startingasync and RunWorkerCompleted. I looked up endinvoke and didn't come up with much as how to use it

[Uranium-235]

ok I *think* I finally found a good workaround. I private'd the return xml reader. I added a timer. Started the timer, put the xml parser stuff in the timer, and have just the HTTP stuff in the background worker (its the only thing that creates pausing, the webclient call). Inside the timer, I have ...If not backgroundworker.isbusy...do all the xml parsing, put messages in the a single collection, and print out each message, clear the collection, than call the backgroundworker at the end of the bottom of the timer

so far, it only gets a busy timer every few seconds, but thats fine, cause that means there will be no missed messages

have no trys/catch's and get no exceptions suprisingly. we'll see how further this will work

[Joacim Andersson]

You don't need a timer to do that. You just need the BackgroundWorker to invoke a method on the GUI thread when it has finished it's job. Let's say you have this method:

  Private Sub ParseXml()
    'Put your timer code in here
  End Sub

The above will need to be called on the UI thread because it tries to access controls on your Form. So in the BackgroundWorker, when it has finished whatever it does and it's time for it to call the above Sub you do this:

    If Me.InvokeRequired Then
      Me.Invoke(New Action(AddressOf ParseXml))
    End If

I used "Me" to reference the Form. You can "reference" the InvokeRequired and Invoke methods of a control from another thread.

[Joacim Andersson]

Sorry, I just noticed that you're using VS 2005 which doesn't have the predefined Action and Func delegates (at least I don't think it has those). You have to first create a delegate. Somewhere in your declaration section of your form, put this line:

  Private Delegate Sub ParseXmlDelegate()

Now to do the invoke, change the code I showed you above to this:

    If Me.InvokeRequired Then
      Me.Invoke(New ParseXmlDelegate(AddressOf ParseXml))
    End If

[Evil_Giraffe]

The other option is to, as I noted in my first reply, to use the ReportProgress mechanism of the BGW which is specifically designed for updating the UI with progress from the BGW. Progress, in this case, is defined as "another message has been received". Pass that message as the userState object, and the ProgressChanged event fires on the UI thread and lets you update the UI with the message.

It seems singularly pointless to Invoke back to the UI thread from the BGW. Why bother with the BGW if you aren't actually going to use what it provides?

[Joacim Andersson]

Ah, that's true. I haven't used the background worker in ages, I normally just create my threads on the go, but I don't develop Windows Forms apps that much anymore either.

[Uranium-235]

I now think I realize why I was getting the "Value dispose() cannot be called while doing CreateHandle()". I was creating a new webclient on the formclosing and using one post operating. Instead now I use the global webclient and it dosen't do that anymore

I have to use a bgworker cause the problem relates to how the webclient works. it does slight gui freezes, even if its in a non-blocking timer =/

[Evil_Giraffe]

I have to use a bgworker cause the problem relates to how the webclient works. it does slight gui freezes, even if its in a non-blocking timer =/

Just to clarify, what you actually need to do is use the WebClient from a non-UI thread. The BGW is an easy way of doing so, but is by no means the only way.