Is it safe to change pointer data in a vector from another thread?

Everything seems to work, but I'm not sure if this is the best way to do this.

Basically, I have an object that performs asynchronous data retrieval. This object has a vector of pointers that are allocated and de-allocated to the main thread. Using boost functions, a process result callback is associated with one of the pointers in this vector. When it fires, it will work on some arbitrary thread and modify the pointer data.

Now I have critical sections around parts that are inserted into the vector and erased if the asynchronous request search object receives more requests, but I wonder if I need some kind of guard in the callback that modifies the pointer data as well.

We hope this pseudo-code with a reduced alias makes it all the more clear:

class CAsyncRetriever
{
    // typedefs of boost functions

    class DataObject
    {
         // methods and members
    };

public:
    // Start single asynch retrieve with completion callback
    void Start(SomeArgs)
    {
        SetupRetrieve(SomeArgs);
        LaunchRetrieves();
    }

protected:
    void SetupRetrieve(SomeArgs)
    {
            // ...

        { // scope for data lock
            boost::lock_guard<boost::mutex> lock(m_dataMutex);
            m_inProgress.push_back(SmartPtr<DataObject>(new DataObject)));
            m_callback = boost::bind(&CAsyncRetriever::ProcessResults, this, _1, m_inProgress.back());
        }

            // ...
    }

    void ProcessResults(DataObject* data)
    {
                // CALLED ON ANOTHER THREAD ... IS THIS SAFE?
        data->m_SomeMember.SomeMethod();
                data->m_SomeOtherMember = SomeStuff;
    }

    void Cleanup()
    {
                // ...

        { // scope for data lock
            boost::lock_guard<boost::mutex> lock(m_dataMutex);
            while(!m_inProgress.empty() && m_inProgress.front()->IsComplete())
                m_inProgress.erase(m_inProgress.begin());
        }

                // ...
         }

private:
    std::vector<SmartPtr<DataObject>> m_inProgress;
    boost::mutex m_dataMutex;
        // other members
};

Edit: this is the actual code for the ProccessResults callback (plus comments for your benefit)

    void ProcessResults(CRetrieveResults* pRetrieveResults, CRetData* data)
        {
// pRetrieveResults is delayed binding that server passes in when invoking callback in thread pool
// data is raw pointer to ref counted object in vector of main thread (the DataObject* in question)

                // if there was an error set the code on the atomic int in object
            data->m_nErrorCode.Store_Release(pRetrieveResults->GetErrorCode());

                // generic iterator of results bindings for generic sotrage class item
            TPackedDataIterator<GenItem::CBind> dataItr(&pRetrieveResults->m_DataIter);
                // namespace function which will iterate results and initialize generic storage
            GenericStorage::InitializeItems<GenItem>(&data->m_items, dataItr, pRetrieveResults->m_nTotalResultsFound); // this is potentially time consuming depending on the amount of results and amount of columns that were bound in storage class definition (i.e.about 8 seconds for a million equipment items in release)
                // atomic uint32_t that is incremented when kicking off async retrieve
            m_nStarted.Decrement(); // this one is done processing

                // boost function completion callback bound to interface that requested results
            data->m_complete(data->m_items);
        }
+3
source share
3 answers

No, this is not safe.

ProcessResultsworking on a data structure passed to her through DataObject. This means that you have a common state between different threads, and if both threads work with the data structure at the same time, you may have problems.

+2
source

As it turned out, the code Cleanupcan destroy an object for which the callback ProcessResultsis in flight. This will cause problems if you cancel the pointer in the callback.

, m_dataMutex, , SetupRetrieve ( , , ), . m_dataMutex , , . ProcessResults , .

+3

, InterlockedExchangePointer ( Windows). , Linux.

The only consideration then would be if one thread uses an obsolete pointer. Does another thread delete the object that the source pointer points to? If so, you have a specific problem.

0
source

All Articles