Providing multiple instances of a form yet processing events one at a time

Question

I need to be able to let multiple instances of the same form be open as my application can be used in different places at once. On the other hand i need to be able to process the operations during the "OK" event one at a time to ensure data is stored safely and not overwritten by another form instance by accident.

I show my form using the .Show() method as i am using a few delegates in it:

        private void newToolStripMenuItem_Click(object sender, EventArgs e)
    {
        bookingForm = new BookingForm(AddMemberBooking, AddUserBooking, CloseBooking);
        bookingForm.Show();
    }

I have tried to use the mutex to allow only one event of the OK button being pressed happen at a time, i have combined this with a Thread to meet the criteria i need.

When i click on the "OK" button i am given the following error:

"Cross-thread operation not valid: Control 'comboBoxDay' accessed from a thread other than the thread it was created on."

This is the code for my booking form class:

using System;

using System.Collections.Generic; using System.ComponentModel; using System.Data; using System.Drawing; using System.Linq; using System.Text; using System.Windows.Forms; using System.Threading;

namespace Collection { //Allows the class to be serialized [Serializable()]

public delegate void AddMemberBookingMethod(int date, int time, int mNo);
public delegate void AddUserBookingMethod(int date, int time, string fName, string lName, string pCode);
public delegate void CloseBookingFormMethod();


public partial class BookingForm : Form
{
    public CloseBookingFormMethod CloseBookingForm;
    public AddMemberBookingMethod AddMemberBooking;
    public AddUserBookingMethod AddUserBooking;
    private Mutex bookingMut = new Mutex();
    private Thread thread;

    public bool IsUser;

    public BookingForm(AddMemberBookingMethod ambm, AddUserBookingMethod aubm, CloseBookingFormMethod cbfm)
    {
        InitializeComponent();
        AddMemberBooking = ambm;
        AddUserBooking = aubm;
        CloseBookingForm = cbfm;

        checkBoxMember.Checked = true;
        //Control.CheckForIllegalCrossThreadCalls = false;
    }

    private void checkBoxUser_CheckedChanged(object sender, EventArgs e)
    {
        if (checkBoxUser.Checked)
        {
            IsUser = true;
            checkBoxMember.CheckState = CheckState.Unchecked;
            textBoxMNo.Enabled = false;
            textBoxFName.Enabled = true;
            textBoxLName.Enabled = true;
            textBoxPCode.Enabled = true;
        }
        else
        {
            IsUser = false;
            checkBoxMember.CheckState = CheckState.Checked;
            textBoxMNo.Enabled = true;
            textBoxFName.Enabled = false;
            textBoxLName.Enabled = false;
            textBoxPCode.Enabled = false;
        }

    }

    private void checkBoxMember_CheckedChanged(object sender, EventArgs e)
    {
        if (checkBoxMember.Checked)
        {
            IsUser = false;
            checkBoxUser.CheckState = CheckState.Unchecked;
            textBoxFName.Enabled = false;
            textBoxLName.Enabled = false;
            textBoxPCode.Enabled = false;
        }
        else
        {
            IsUser = true;
            checkBoxUser.CheckState = CheckState.Checked;
            textBoxMNo.Enabled = false;
            textBoxFName.Enabled = true;
            textBoxLName.Enabled = true;
            textBoxPCode.Enabled = true;
        }

    }

    private void buttonOK_Click(object sender, EventArgs e)
    {
        this.thread = new Thread(new ThreadStart(MakeBooking));
        this.thread.Name = "bookingThread";
        this.thread.Start();
    }

    private void MakeBooking()
    {
        this.bookingMut.WaitOne();

        int date = this.comboBoxDay.SelectedIndex;
        int time = this.comboBoxTime.SelectedIndex;

        if (IsUser)
        {
            string fName = textBoxFName.Text;
            string lName = textBoxLName.Text;
            string pCode = textBoxPCode.Text;

            AddUserBooking(date, time, fName, lName, pCode);
        }
        else
        {
            int mNo = int.Parse(textBoxMNo.Text);

            AddMemberBooking(date, time, mNo);
        }

        this.bookingMut.ReleaseMutex();

        CloseBookingForm();
    }

    private void buttonClose_Click(object sender, EventArgs e)
    {
        CloseBookingForm();
    }
}

}

I realise i may not be doing this in the most efficient way but time is a bit of a factor. I've researched the error and have heard of using delegates and .Invoke() but im still not entirely sure how to fix it.

Thanks!

EDIT:

I've found this code snippet when searching for a fix to my problem, i don't understand where/how i would use it.

if(this.InvokeRequired)

{ this.Invoke(new MyEventHandler(this.CreateAForm())); return; }

EDIT2:

Seems the guy finally saw sense, by creating the from with the "new" word it apparently passes the criteria, i wish i'd of known this before trying to reinvent the wheel.

Answer 1

I think you could simply disable the OK buttons on other open forms to give users a visual cue. Then you shouldn't even have the issue. Provide a callback delegate to something in the application controller which knows which forms are open. Each form can provide a public method to disable the OK button. Disable to OK button on all the other forms.

I'm not really following your code too well. I would think the mutex could be outside of the form code in the first place (i.e. in the delegates that do the actual work), and if it is within a single application, you could just use the lock (object) method to ensure only one thread is executing a given bit of code.

I'd also like to add that a mutex is not going to stop multiple users on different machiens being able to click OK at the same time. I'm not sure if that's what you meant in your question by a form being run in different places.

I think that AddUserBooking and the other delegate should be responsible for ensuring that they are threadsafe and this should not be part of the UI. If they aren't threadsafe, why aren't they? It's relatively easy to make database commit functions each have their own connection to the database during their operations and thread-safety should not be an issue.

Answer 2

One simple way to do this is to use the overload of the Thread.Start method that accepts an object: Thread.Start Method (Object). In this object you will store all the data/state necessary in order to make the update.

All the code that references the form and its controls needs to be moved into the OK click event method or refactored out to a method that just returns a data object. Then pass this object into the thread start method.

Some pseudo code:

on_click_event()
{
   object data=getFormData();
   thread.start(data);
}

There are better ways to do this but this is a quick fix for your code.

Answer 3

You are getting this exception because your thread is accessing controls. That's not legal, control properties must only ever be accessed from the UI thread. You're okay on the TextBox.Text property, that one happens to be cached. But not ComboBox.SelectedIndex. And closing the form from another thread is going to bomb too.

Your mutex has nothing to do with it, but keep it if you want to prevent threads from overlapping. Using a delegate's Invoke method isn't going to solve it, that just starts a thread as well. You'll need to collect the info that the thread is going to need in a little helper class and pass that as the argument to the Thread.Start() method.

Closing the form is a bit tricky too, the user might well have already closed it while the thread was running. That's going to cause an ObjectDisposed exception. A quick fix is to set the form's Enabled property to false so the user can't close it. You'll need to use the form's Invoke() method to ensure the closing is done on the UI thread.

Last but not least, if these threads don't take a lot of time (a second or so), consider not using threads at all and display a wait cursor instead.