2

First of all, the code below seems to be working. It extracts jpeg images from a continuous byte stream and displays them in a pictureBox as they arrive if the encapsulating packet checksum is correct. The concern is intermittent GUI problems since the pictureBox is asynchronously updated by RxThread. Is the method used here OK or might this crash while showing it to the customer?

public FormMain()
{
    InitializeComponent();
    var t1 = new Thread(RxThread) { IsBackground = true };
    t1.Start();
}

private void RxThread()
{
    while (true)
    {
        ... // validate incoming stream
        var payload = new Byte[payloadSize];
        ... // copy jpeg image from stream to payload
        pictureBox.Image = new Bitmap(new MemoryStream(payload));
    }
}
jacknad
  • 13,483
  • 40
  • 124
  • 194
  • 1
    It is not evil. You merely pointed the gun at your foot and pulled the trigger. Nobody is worried about you hopping around. – Hans Passant Jun 29 '11 at 23:33

3 Answers3

4

I think all access to UI controls should be done from UI thread. Modifying control from the thread that doesn't own the underlying handle may have undesirable effects. In the best case scenario the exception will be thrown, in the worst case everything may seem to be all right until some race condition happens (and you may spend lots of time trying to replicate it).

Use Invoke method, passing your delegate that will be executed on the UI thread.

Jakub Konecki
  • 45,581
  • 7
  • 87
  • 126
3

Why you don't use Invoke to update the PictureBox?

mohamede1945
  • 7,092
  • 6
  • 47
  • 61
2

Are you sure that even works at all? I don't see why it wouldn't raise a InvalidOperationException: (Cross-thread operation not valid) as the control is being updated from a thread other than the one which is was created on. You should update the UI via a delegate method that is invoked on the UI thread.

Ed S.
  • 122,712
  • 22
  • 185
  • 265
  • That only happens when running from the IDE, if you run outside the IDE without debugging then I'm fairly certain they get suppressed. Same goes for a bunch of other "oh noes, this is bad programming" style errors. So although its a bad way to do it, unfortunately people can ignore the warning by running outside the IDE. Although they'll still get the random crashes (I've had plenty of experience with those when I first started using threading, haha) – William Lawn Stewart Jun 29 '11 at 22:19
  • 1
    I wasn't aware of that, but this is not an "oh noes, this is bad programming" issue; this is a real issue that causes real problems at runtime. This *is* bad programming and you should always update controls from the thread they were created on. – Ed S. Jun 29 '11 at 22:21
  • Exactly, which is why I think its silly that it ignores the problem when running without the IDE. But that's probably why he said it seems to work fine. – William Lawn Stewart Jun 29 '11 at 22:22
  • @William: Oh, ok, sorry; I inferred something different than what you actually meant from that comment =). Yes, it really should crash at runtime, but I'm betting this decision was made to maintain compatibility. This exception was not raised at all in .NET 1.1 if I remember correctly. – Ed S. Jun 29 '11 at 22:24