Problems with wxThreads Topic is solved

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
HeReSY
Earned some good credits
Earned some good credits
Posts: 120
Joined: Fri Sep 17, 2004 8:58 pm
Location: Germany

Problems with wxThreads

Post by HeReSY »

Hi,

i have a problem with two threads in my application.
In that i use a global vector to store some data. Both vectors have access to this vector. But when i want to delete some data from the vector, my application crashes.

Code: Select all

typedef vector<TSSTatus*> PtrStatusList;
// My global List
PtrStatusList g_StatusList;

Code: Select all

while(!TestDestroy())
    {
        // Get some information from the serial port.

        if(readedBytes > 0)
        {
            wxString protocol = _("");
            receivedString[readedBytes] = 0;
            wxString temp(receivedString, readedBytes);
            m_sInputBuffer << temp;
            // Create a protocol from the input
            TSProtocol *prot = Processing(m_sInputBuffer);
            if(!protocol.IsEmpty())
            {
                int size = g_StatusList.size();
                PtrStatusList::iterator it = g_StatusList.end();
                for(int i = size; i > 0; i--)
                {
                    wxString temp;
                    temp << i << (". Listeneintrag");
                    LogMessage(temp);
                    TSStatus *stat = g_StatusList.at(i-1);
                    stat->Response(prot);
                    // When i receive an ACK, then the object should be deletet
                    if(stat->IsAck())
                    {
                       // Here my Application crashes
                        g_StatusList.erase(it);
                        delete stat;
                    }
                    it--;
                }
            }
            delete prot;
        }
    }

Code: Select all

while(!TestDestroy())
    {
        int size = g_StatusList.size();
        if(!size) continue;
        for(PtrStatusList::iterator it = g_StatusList.end();
            it != g_StatusList.begin(); it--)
        {
            size--;
            TSStatus *stat = g_StatusList.at(size);
            if(!m_bWait)
            {
                if((stat->IsTimeout() || stat->IsNack()) && stat->GetCounter() < 3)
                {
                    wxString string = stat->GetDataToSend();
                    m_pSerPort->Send(string);
                }
            }
            stat->Increment();
        }
        Sleep(100);
    }
I hope someone can help me.

HeReSY
lvdlinden
Earned some good credits
Earned some good credits
Posts: 147
Joined: Thu May 17, 2007 7:03 am
Location: 's-Hertogenbosch, Netherlands

Post by lvdlinden »

Hello Heresy,

The threads should synchronize their access to g_StatusList with a mutex, because the second thread may try to access an element that was just deleted by the first.

I haven't ran your code, so I am just guessing:

Code: Select all

if(!protocol.IsEmpty())
{
    int size = g_StatusList.size();
    PtrStatusList::iterator it = g_StatusList.end(); // <-- it points to element after the last one
    for(int i = size; i > 0; i--)
    {
        ...
        TSStatus *stat = g_StatusList.at(i-1);
        ...
        // When i receive an ACK, then the object should be deletet
        if(stat->IsAck())
        {
            ...
            // Here my Application crashes
            g_StatusList.erase(it); // <-- could it still be g_StatusList.end()?
                        delete stat;
                    }
                    it--;
                }
Erasing an element may invalidate any iterators you hold to a vector. You may want to consider using std::list, where iterators will still be valid after an erase. Or you could look into std::remove_if.

Also, your second thread enters a busy-wait loop while g_StatusList.size() == 0. The Sleep(100) is never reached because of the continue.
tan
wxWorld Domination!
wxWorld Domination!
Posts: 1471
Joined: Tue Nov 14, 2006 7:58 am
Location: Saint-Petersburg, Russia

Re: Problems with wxThreads

Post by tan »

Hi,
at first sight:
HeReSY wrote:

Code: Select all

while(!TestDestroy())
    {
        // Get some information from the serial port.

        if(readedBytes > 0)
        {
            wxString protocol = _("");
            receivedString[readedBytes] = 0;
            wxString temp(receivedString, readedBytes);
            m_sInputBuffer << temp;
            // Create a protocol from the input
            TSProtocol *prot = Processing(m_sInputBuffer);
            if(!protocol.IsEmpty())
            {
                int size = g_StatusList.size();
                PtrStatusList::iterator it = g_StatusList.end(); // <-- it is invalid
                // if you want ptr to LAST valid entry, add line: it--;
                // Or move this line in the loop at the beginning of the loop.
                for(int i = size; i > 0; i--)
                {
                    wxString temp;
                    temp << i << (". Listeneintrag");
                    LogMessage(temp);
                    TSStatus *stat = g_StatusList.at(i-1);
                    stat->Response(prot);
                    // When i receive an ACK, then the object should be deletet
                    if(stat->IsAck())
                    {
                       // Here my Application crashes
                        g_StatusList.erase(it);
                        delete stat;
                    }
                    it--;
                }
            }
            delete prot;
        }
    }
HeReSY
OS: Windows XP Pro
Compiler: MSVC++ 7.1
wxWidgets: 2.8.10
HeReSY
Earned some good credits
Earned some good credits
Posts: 120
Joined: Fri Sep 17, 2004 8:58 pm
Location: Germany

Post by HeReSY »

Thanks tan, that solves one of my problems.

Now there is the only problem, that not both threads have access to the list.

HeReSY
tan
wxWorld Domination!
wxWorld Domination!
Posts: 1471
Joined: Tue Nov 14, 2006 7:58 am
Location: Saint-Petersburg, Russia

Post by tan »

HeReSY wrote: Now there is the only problem, that not both threads have access to the list.
HeReSY
use mutex, as suggests lvdlinden:

Code: Select all

// My global List
PtrStatusList g_StatusList;
wxMutex  g_lock;

...
            if(!protocol.IsEmpty())
            {
               g_lock.Lock();
               // Do some work
               g_lock.Unlock();
            }
...

while(!TestDestroy())
    {
        g_lock.Lock();
        int size = g_StatusList.size();
        if(size)
        {
             // Do some work
        }
        g_lock.Unlock();
        Sleep(100);
    }
OS: Windows XP Pro
Compiler: MSVC++ 7.1
wxWidgets: 2.8.10
Post Reply