lambdac++buildertthread

Passing parameters to TThread::Queue using lambda in C++ Builder


I want to do something like this:

void TForm2::SetMsg(UnicodeString fMsg)
{
// If thread is not main Queue...
if (GetCurrentThreadId() != System::MainThreadID)
    {
    TThread::Queue( nullptr, [&]() -> void { Form2->SetMsg(fMsg); } );
    return;
    }

// If thread is main then print directly...
Memo1->Lines->Add(fMsg);
}

So I simply call SetMsg("mymessage"); and regardless in which thread it is called from it prints properly. The above would work except the UnicodeString gets deallocated before lambda is called.

It is my understanding that the lambda above should remain allocated in this form, and it indeed works if I hardcode the string in the lambda itself instead of fMsg parameter. So how do I pass the parameter then?

I could allocate it with new and delete later, but that fails for some reason too.

I tried:

void TForm2::SetMsg(UnicodeString fMsg)
{
// If thread is not main Queue...
if (GetCurrentThreadId() != System::MainThreadID)
    {
    UnicodeString* p = new UnicodeString(fMsg);
    TThread::Queue( nullptr, [&]() -> void { try { Form2->SetMsg(*p); } __finally { delete p; } }  );
    return;
    }

// If thread is main then print directly...
Memo1->Lines->Add(fMsg);
}

Would it be faster (less expensive) or easier with the TThread::CreateAnonymousThread ?

The first version works fine if TThread::Synchronize is used but I want to avoid thread blocking.


Solution

  • The above would work except the UnicodeString gets deallocated before lambda is called.

    Your lambda is capturing the UnicodeString by reference, so when SetMsg() exits, the UnicodeString will be destroyed, leaving the reference inside the lambda dangling. The lambda needs to capture the UnicodeString by value instead to make its own copy.

    Also, I would not recommend using the global Form2 variable to call SetMsg() on, I would have the lambda capture SetMsg's this pointer instead.

    Try this:

    void TForm2::SetMsg(UnicodeString fMsg)
    {
        // If thread is not main Queue...
        if (GetCurrentThreadId() != System::MainThreadID)
        {
            TThread::Queue( nullptr, [this, fMsg]{ this->SetMsg(fMsg); } );
            return;
        }
    
        // If thread is main then print directly...
        Memo1->Lines->Add(fMsg);
    }
    

    Alternatively, don't have the lambda call SetMsg() a second time, just access the Memo directly instead, since you know the GetCurrentThreadId() check will be redundant at that point:

    void TForm2::SetMsg(UnicodeString fMsg)
    {
        // If thread is not main Queue...
        if (GetCurrentThreadId() != System::MainThreadID)
        {
            TThread::Queue( nullptr, [this, fMsg]{ this->Memo1->Lines->Add(fMsg); } );
            return;
        }
    
        // If thread is main then print directly...
        Memo1->Lines->Add(fMsg);
    }
    

    I could allocate it with new and delete later, but that fails for some reason too.

    Your lambda is capturing the pointer by reference, so you are leaving the reference inside the lambda dangling when SetMsg() exits. You need to capture the pointer by value instead:

    void TForm2::SetMsg(UnicodeString fMsg)
    {
        // If thread is not main Queue...
        if (GetCurrentThreadId() != System::MainThreadID)
        {
            UnicodeString* p = new UnicodeString(fMsg);
            TThread::Queue( nullptr, [this, p]{ try { this->SetMsg(*p); /* or: this->Memo1->Lines->Add(*p); */ } __finally { delete p; } } );
            return;
        }
    
        // If thread is main then print directly...
        Memo1->Lines->Add(fMsg);
    }
    

    In general, using a dynamic UnicodeString will work, though I would strongly suggest capturing a std::unique_ptr or std::shared_ptr instead to handle the deallocation for you, instead of using a manual try/__finally block:

    void TForm2::SetMsg(UnicodeString fMsg)
    {
        // If thread is not main Queue...
        if (GetCurrentThreadId() != System::MainThreadID)
        {
            // using unique_ptr, must be moved into the lambda
            auto p = std::make_unique<UnicodeString>(fMsg);
            TThread::Queue( nullptr, [this, p = move(p)]{ this->SetMsg(*p); /* or: this->Memo1->Lines->Add(*p); */ } );
    
            // using shared_ptr, can be copied by value
            auto p = std::make_shared<UnicodeString>(fMsg);
            TThread::Queue( nullptr, [this, p]{ this->SetMsg(*p); /* or: this->Memo1->Lines->Add(*p); */ } );
    
            return;
        }
    
        // If thread is main then print directly...
        Memo1->Lines->Add(fMsg);
    }
    

    Would it be faster (less expensive) or easier with the TThread::CreateAnonymousThread?

    No. You are already running in a thread when calling Queue(), there is no reason to waste overhead spawning another thread. And you would still have to deal with copying values into that thread's lambda/callback function.