CODEWITHSUNDEEP
c++multithreadingclassvector
VirusEcks
VirusEcks

Reputation: 596

C++ Vectors / Classes /Threading

I'm Using Win7 / VS2008 (9) SDK 6/7.1

i faced a problem in a code that i'm using
the mini version of the code as follows 

class CONNECTION
{
    int/std::string/bool vars; // just to make it simple
    CONNECTION ( int defaultvar );
    CONNECTION ( const CONNECTION& copycon )
    ~CONNECTION ( );
    DWORD static WINAPI staticstart( void *param )  //HACK to use createthread on classes
    { return ((CONNECTION *)param)->main); } // yea it works fine
    DWORD main();
};

this class has no default constructor with a copy and a destructor all vars are copied fine and destructor leaves no memleaks, constructor is simple as assigning the paramters to a var .. all vars and code are omitted to make it simple and because they aren't the prob. 

class main
{
    std::vector<CONNECTION> con;
    int addcon( int defaultvarofcon )
    {
        CONNECTION temp( defaultvar );
        con.push_back( temp );
        return con.size() - 1;
    }
}

so far so good when i run a console test program that only have an include and this code

main mymainclass;
mymainclass.addcon( 0 );

program runs fine closes without errors
but when i add extra code like 

main mymainclass;
mymainclass.addcon( 0 );
mymainclass.addcon( 1 );
mymainclass.addcon( 2 );

program crashes with access violation after checking my code twice i debugged it step by step on all threads i found out the main thread can read correct values of all vector class/elements in both main thread and worker thread ONLY if i'm using one element at the vector

however if i'm using more than one like the second code all data on all elements on their own threads are inaccessible ( bad pointers ) . but on main thread they still correct and showing right values

can any one please help me figure out what's wrong with this code ?

Upvotes: 1

Views: 653

Answers (2)

Ben Voigt
Ben Voigt

Reputation: 283921

std::vector is not designed to be thread safe. Therefore you have to use some sort of mutex to make sure only one thread accesses it at a time.

Otherwise, if any thread resizes the vector, the vector may have to allocate new storage, copy elements, then free the old storage... and clearly freeing the old storage while some other thread is using it is a very bad thing.

You need to allocate the CONNECTION objects yourself. Letting std::vector manage them means they are going to get moved around in memory when the vector resizes, and the pointers held by other threads are left dangling. Resizing a vector invalidates all pointers to any of its content.

Upvotes: 2

sth
sth

Reputation: 229914

If you need a custom destructor and copy constructor, you probably also need a custom assignment operator.

Without that you quickly end up with several instances of your class referring to the same internal pointers/..., easily leading to memory corruption and access violations.

Upvotes: 1

Related Questions