parallel function call and thread issue

Question

Sorry to post one issue here which is related to code. i was debugging a code which was wrote long time back by one developer. the issue is two function is called parallel using thread and one function set a variable true at end and another function just looping until the variable is set to true. the variable value is not getting set to true. here i am posting a code snippet and just tell me what is mistake in the code for which variable is not getting set to true by first function.

when user click button then two function is invoked

       private void UPDATE_Click(object sender, System.EventArgs e)
        {
            SavedFirst();
            AddCheckThread();
        }

public void SavedFirst()
        {
                IsOpen = true;              
                System.Threading.Thread loadT = new System.Threading.Thread(new System.Threading.ThreadStart(SaveAll));
                loadT.Start();
                IsOpen = false;
        }

        private void AddCheckThread()
        {
            if (!ALLFlag)
            {
                loadingThread = new System.Threading.Thread(new System.Threading.ThreadStart(addCheck));
                loadingThread.Priority = System.Threading.ThreadPriority.Lowest;
                loadingThread.Start();
            }
        }

        private void SaveAll()
        {
            if (this.InvokeRequired)
            {
                this.Invoke(new MethodInvoker(delegate
                {
                    ALLFlag = false;
                    if (!ALLFlag)
                    {
                        loadingThread = new System.Threading.Thread(new System.Threading.ThreadStart(AddProducts));
                        loadingThread.Priority = System.Threading.ThreadPriority.Lowest;
                        loadingThread.Start();
                    }

                }));
                return;
            }
        }

        private void AddProducts()
        {
            if (this.InvokeRequired)
            {
                this.Invoke(new MethodInvoker(delegate
                {
                    ALLFlag = false;
                    if (System.Windows.Forms.MessageBox.Show(this, "Would you like to add the details into the Database?", "Add?", System.Windows.Forms.MessageBoxButtons.YesNo) == System.Windows.Forms.DialogResult.Yes)
                    {
                        if (comboOUR.SelectedItem == null || comboCountry.SelectedItem == null || comboSelleble.SelectedItem == null || txtOereff.Text == "" || txtUKPrice.Text == "" || txtUSPrice.Text == "" || txtSUrCharge.Text == "" || txtOURUS.Text == "" || txtOURUK.Text == "")
                        {
                            FormValidation();
                        }
                        else
                        {
                            Gather_Data();
                            bool isInserted = false;
                            if (System.Windows.Forms.MessageBox.Show(this, "Would you like to add the details into \"Detailed-Product\" Too?", "Add?", System.Windows.Forms.MessageBoxButtons.YesNo) == System.Windows.Forms.DialogResult.Yes)
                            {
                                isInserted = bbaProduct.BBASaveProduct();
                                if (isInserted == true)
                                {
                                    isInserted = false;
                                    isInserted = bbaProduct.InsertInProductTable(BBAProduct.DetailProductID);
                                    if (isInserted == true)
                                    {
                                        System.Windows.Forms.MessageBox.Show(this, "Product Successfully Added ", "Add");
                                    }
                                    else
                                    {
                                        System.Windows.Forms.MessageBox.Show(this, "Error Occurred !! Not Added Into the Database ", "Add");
                                    }
                                }
                                else
                                {
                                    System.Windows.Forms.MessageBox.Show(this, "Error Occurred !! Not Added Into the Database ", "Add");
                                }
                            }
                            else
                            {
                                isInserted = bbaProduct.InsertInProductTable(0);
                                if (isInserted == true)
                                {
                                    System.Windows.Forms.MessageBox.Show(this, "Successfully Inserted Into the database", "Add");
                                }
                                else
                                {
                                    System.Windows.Forms.MessageBox.Show(this, "Error Occurred !! Not Added Into the Database", "Add");
                                }

                            }
                        }
                    }
                    else
                    {
                        System.Windows.Forms.MessageBox.Show(this, "Process Cancelled By The User", "Add");
                    }
                    ALLFlag = true;
                }));
                return;
            }
        }

        #region Add Check

        private void addCheck()
        {
            if (this.InvokeRequired)
            {
                this.Invoke(new MethodInvoker(delegate
                {
                    this.Opacity = 0.8;

                    axShockwaveFlash1.Visible = true;
                    axShockwaveFlash1.Play();
                    while (!ALLFlag)
                    {

                        int x = 0;
                    }
                    axShockwaveFlash1.Visible = false;
                    axShockwaveFlash1.Visible = false;
                    axShockwaveFlash1.StopPlay();
                    this.Opacity = 1;
                    ALLFlag = false;
                    loadingThread.Abort();
                }));
                return;
            }
        }

help me to locate where is the mistake for which ALLFlag value is not getting set to true. thanks

It would help if you could come up with a short but complete example demonstrating the problem (ideally with better formatting, too). I would strongly discourage you from using this sort of tight loop anyway though, to be honest... it would be much better to start a `Task` and wait for that to complete. (It doesn't help that we don't even know what `this` is, so we can't tell what `this.Invoke` is doing... If it's a form, so you're using the Windows Forms UI thread, you shouldn't be blocking that...) — Jon Skeet, Mar 25 '15 at 14:04
There's no point in continually creating new threads that do nothing but marshal right back to the UI thread. You'd get the same outcome by simply doing all of the work right in the UI thread. — Servy, Mar 25 '15 at 14:05
It is really hard to read this code. Is `ALLFlag` declared as `volatile`? The other threads may not see updates made to this field. — nikis, Mar 25 '15 at 14:10
@nikis Personally I agree but the jury is still out on that: http://stackoverflow.com/questions/4838828/why-should-a-function-have-only-one-exit-point — Einer, Mar 25 '15 at 14:19

George Mamaladze · Accepted Answer · 2015-03-25T14:27:13.580

First of all try to understand what the lagacy code is trying to accomplish.

I think this is one of very common situations where one thread is waiting for another to accomplish some task. Your forerunner has chosen most naive, but very wrong pattern to deal with that:

while (!ALLFlag)
{
  int x = 0;
}

this is the code place where she is trying to let one thread pause working until another sets the flag.

The state of the art solution for that would be to use a EventWaitHandle or one of it's variants.

On the other as more as I look at your code I think you need to spend significant time in understanding which are tasks you are trying to accomplish parallely, who starts them and when, and which are shared resources and synchronization points between them. I do not think this one flag is a single problem of that code.

parallel function call and thread issue

1 Answers1