I am working on a WinForms app consisting of several custom UserControls.

One of these custom controls called ProgressTracker shows progress of 3 different types using labels. It displays a text message, a stage counter, and precentage counter. It has two sub-classes called ValuesTracker and Locker.

Locker class works like mutex. It uses Lock(), IsLocked(), and Unlock() methods as shown below.

    private: ref class Locker
    {
    public:
        void Lock(bool ForceUnlock)
        {
            if (ForceUnlock && IsLocked()) return;

            while (true)
            {
                if (!IsLocked())
                {
                    System::Threading::Thread::VolatileWrite(_locked, 1);
                    return;
                }
            }
        }
        void Unlock()
        {
            System::Threading::Thread::VolatileWrite(_locked, -1);
        }
        bool IsLocked()
        {
            int x = System::Threading::Thread::VolatileRead(_locked);
            if (x == 1) return true; else return false;
        }

    private:
        int _locked = -1;
    };

The tracker class keeps the Current and Maximum integer counters and manipulate the values using System::Threading::Thread::VolatileRead and System::Threading::Thread::VolatileWrite.

    public:  ref class ValuesTracker
    {
    private:
        ref class ThrObj
        {
        public:
            ThrObj(int Value, bool Progress_Changed)
            {
                val = Value;
                ProgressChanged = Progress_Changed;
            }
            int val;
            bool ProgressChanged;
        };

        int _Current;
        int _Maximum;

        ProgressTracker^ Owner;
        Locker^ lock = gcnew Locker;

        void SetCurrent(System::Object^ obj)
        {
            ThrObj^ _obj = (ThrObj^)obj;

            if (_obj->val < 0) { _obj->val = 0; }
            else { int max = GetMaximum(); if (_obj->val > max) _obj->val = max; }

            System::Threading::Thread::VolatileWrite(_Current, _obj->val);

            lock->Unlock();

            if (_obj->ProgressChanged) Owner->UpdateUI();
        }
        void SetMaximum(System::Object^ obj)
        {
            ThrObj^ _obj = (ThrObj^)obj;

            if (_obj->val < 0) { _obj->val = 0; }
            else { int min = GetCurrent(); if (_obj->val < min) _obj->val = min; }

            System::Threading::Thread::VolatileWrite(_Maximum, _obj->val);

            lock->Unlock();

            if (_obj->ProgressChanged) Owner->UpdateUI();
        }

    public:
        ValuesTracker(ProgressTracker^ _Owner_, int _Current_, int _Maximum_)
        {
            if (_Current_ > _Maximum_) _Current_ = _Maximum_;

            _Current = _Current_;
            _Maximum = _Maximum_;

            Owner = _Owner_;
        }

        void SetCurrent(int Value, bool TriggerProgressChanged)
        {
            lock->Lock(false);

            System::Threading::Thread^ thr = gcnew System::Threading::Thread
            (gcnew System::Threading::ParameterizedThreadStart(this, &ValuesTracker::SetCurrent));
            thr->IsBackground = true;
            thr->Start(gcnew ThrObj(Value, TriggerProgressChanged));
        }
        void SetMaximum(int Value, bool TriggerProgressChanged)
        {
            lock->Lock(false);

            System::Threading::Thread^ thr = gcnew System::Threading::Thread
            (gcnew System::Threading::ParameterizedThreadStart(this, &ValuesTracker::SetMaximum));
            thr->IsBackground = true;
            thr->Start(gcnew ThrObj(Value, TriggerProgressChanged));
        }

        int GetCurrent()
        {
            return System::Threading::Thread::VolatileRead(_Current);
        }
        int GetMaximum()
        {
            return System::Threading::Thread::VolatileRead(_Maximum);
        }
    };

I was testing the tracker today using a for loop to simulate percentage increment and called System::Threading::Thread::Sleep(300) after every iteration. To my surprise, it did not work. Below is the method UpdateUI() from the class ProgressTracker and the sub-methods it calls.

    public:  void UpdateUI()
    {
        if (lock->IsLocked()) return;

        if (!full_init) return;

        lock->Lock(false);

        System::Threading::Thread^ thr = gcnew System::Threading::Thread
        (gcnew System::Threading::ThreadStart(this, &ProgressTracker::UpdateUI_sub1));
        thr->IsBackground = true;
        thr->Start();
    }
    private: void UpdateUI_sub1()
    {
        this->Invoke(gcnew System::Windows::Forms::MethodInvoker(this, &ProgressTracker::UpdateUI_sub2));
    }
    private: void UpdateUI_sub2()
    {
        if (_StatusMessageChanged) { label_1_Status->Text = _StatusMessage; _StatusMessageChanged = false; }
        label_Stage->Text = Stage->GetCurrent().ToString()   L"/"   Stage->GetMaximum().ToString();
        label_Percentage->Text = Percentage->GetCurrent().ToString()   L"%";
        lock->Unlock();
    }

The reason I am using so many threads is because the called methods in those threads are really small and I think will not put much strain on the system. Anyways, after diagnosing and troubleshooting, I found that all the methods were being called and instructions being executed. The problem occurs when thread reaches the execution of the instruction that calls the setter of Text property of labels in the UpdateUI_sub2() method. This instruction is not executed until "return" is executed from the main method of the parent that I am using to test the ProgressTracker as below.

System::Void win_Main::SourceCheckClicked(System::Object^ sender, System::EventArgs^ e)
{
    progressTracker1->Percentage->SetMaximum(10, true);
    for (int i = 1; i <= 10; i  )
    {
        System::Console::WriteLine(i);
        progressTracker1->Percentage->SetCurrent(i, false);

        System::Threading::Thread::Sleep(300);
    }

    System::Windows::Forms::RadioButton^ chk = (System::Windows::Forms::RadioButton^)sender;

    if (chk->Checked) return;

    SetCheckState(check_Source_Free, false);
    SetCheckState(check_Source_Paid, false);
    SetCheckState(check_Source_Server, false);

    if (chk->Name == L"check_Source_Free") SetCheckState(check_Source_Free, true);
    else if (chk->Name == L"check_Source_Paid") SetCheckState(check_Source_Paid, true);
    else if (chk->Name == L"check_Source_Server") SetCheckState(check_Source_Server, true);
}

I do not know what I am doing wrong here. I also think that this whole code can be improved in a lot of ways. Please suggest your best.

CodePudding user response：

I have figured out the problem. The event completion method SourceCheckClicked is called on the main thread. This main thread is also responsible for updating UI. Therefore the UI update requests are getting blocked until return has been called. The correct way to do this is to do the processing in a background thread and send requests to update the GUI from there.