occasionally problems with invokerequired in c#

Question

I'm having problems with the invokerequired method from time to time and wanted to aks if someone knows that problem or can it explain to me. I have some code:

 splashScreen splashObj = splashScreen.GetInstance();

        if (thrd == null)
        {
            thrd = new Thread(new ThreadStart(loadingScreenStart));
            thrd.IsBackground = true;
            thrd.Start();
        }

        else
        {
            if (splashObj.InvokeRequired)
            {
                splashObj.Invoke((MethodInvoker)delegate()
                {
                    splashObj.Show();
                }
                );
            }
            else
            {
                splashObj.Show();
            }
        }

        if (splashObj.loadLabel.InvokeRequired)
        {
            splashObj.loadLabel.Invoke((MethodInvoker)delegate()
            {
                splashObj.loadLabel.Text = "Checking Username...";
            }
            );
        }
        else
        {
            splashObj.loadLabel.Text = "Checking Username...";
        }


  public void loadingScreenStart()
    {
        splashScreen splashObj = splashScreen.GetInstance();
        Application.Run(splashScreen.GetInstance());
    }

the splashscreen code:

 public partial class splashScreen : Form
{

    public Form1 form1;

    public splashScreen()
    {
        InitializeComponent();
        //form1 = frm1;
    }

    public splashScreen(Form1 frm1)
    {
        form1 = frm1;
    }


    private void splashScreen_Load(object sender, EventArgs e)
    {

    }

    private static splashScreen m_instance = null;
    private static object m_instanceLock = new object();

    public static splashScreen GetInstance()
    {

        lock (m_instanceLock)
        {
            if (m_instance == null)
            {
                m_instance = new splashScreen();
            }
        }

        return m_instance;
    }
}

So I'm showing a splashscreen while the rest of the code is loading in the background. Messages are getting showed like "Checking Username...", "Checking Password...", "Loading..." and so on. This code works fine but from time to time it seems like the code execution is faster than the thread and then I get an error that the method was called from an other thread than the one it was created on. Why does this happen? Is there any solution for this kind of problem? That happens maybe once in twenty executions.

Answer 1

Multi-threading is hard, so you should try making it as simple as possible. One, there's no reason to have any of the splashscreen code outside of the splashscreen class itself. Second, you should always know what you're doing, so InvokeRequired is just a code smell that says "I have no idea who's going to call this method".

void Main()
{
    SplashScreen.ShowText("Loading 1");
    Thread.Sleep(1000);
    SplashScreen.ShowText("Loading 2");
    Thread.Sleep(2000);
    SplashScreen.Done();
    Thread.Sleep(2000);
    SplashScreen.ShowText("Loading 3");
}

// Define other methods and classes here
public partial class SplashScreen : Form
{
  private static SplashScreen instance;
  private static readonly ManualResetEvent initEvent = new ManualResetEvent(false);

  Label loadLabel;

  private SplashScreen()
  {
    // InitializeComponent();
    loadLabel = new Label();
    Controls.Add(loadLabel);

    Load += (s, e) => initEvent.Set();
    Closing += (s, e) => initEvent.Reset();
  }

  private static object syncObject = new object();
  private static void InitializeIfRequired()
  {
    // If not set, we'll have to init the message loop
    if (!initEvent.WaitOne(0))
    {
      lock (syncObject)
      {
        // Someone initialized it before us
        if (initEvent.WaitOne(0)) return;

        // Recreate the form if it was closed
        instance = new SplashScreen();

        var thread = new Thread(() => { Application.Run(instance); });
        thread.Start();

        // Wait until the form is ready
        initEvent.WaitOne();
      }
    }
  }

  public static void ShowText(string text)
  {
    InitializeIfRequired();

    instance.Invoke((Action)(() => 
     { 
       if (!instance.IsDisposed) instance.loadLabel.Text = text; 
     }
    ));
  }

  public static void Done()
  {
    // Is it closed already?
    if (!initEvent.WaitOne(0)) return;

    lock (syncObject)
    {
      // Someone closed it before us
      if (!initEvent.WaitOne(0)) return;

      instance.Invoke((Action)(() => { instance.Close(); }));
    }
  }
}

This is a snippet for LINQPad, you'll want to remove the comment on InitializeComponent as well as the loadLabel control (which should be on the designer instead).

This way, you have the logic of the splash screen completely isolated from the rest of the application. To show a splashscreen text, just call SplashScreen.ShowText. To make the splashscreen go away, call SplashScreen.Done.

Note how there's no need to use InvokeRequired - there's no way any legitimate call of SplashScreen.ShowText (or Done) would ever not need marshalling to the UI thread of the splash screen.

Now, this is not perfect. It's something I wrote in about 10 minutes. But it's (probably :)) thread-safe, follows best practices a lot better than the original, and is much easier to use.

Also, there's cases where I would use higher-level constructs, e.g. Lazy<T> and Task - but since that wouldn't really help here (starting the new messaging loop, having to recreate the form if it was closed...), I opted for the simpler solution instead.

Note that I'm using Invoke rather than BeginInvoke or similar - this is quite important, because otherwise it would be possible to queue a close, followed by a ShowText that would work on a disposed form. If you intend to call ShowText from multiple threads, it would be safer to also lock around the whole ShowText body. Given the usual use case for splash screens, there's some thread-safety that's unnecessary, but...