Problem with constructors w/ threads (boost)

[Qqite]

So I'm having a very odd problem that I must be overlooking. I've been stuck on this for a couple of hours and hopefully its just a stupid mistake.

I have a VECTOR of instanced classes that in the constructor start a thread for that object.

Heres the part of the code that (I think) is the problem).
Its the constructor for the class that is being put in a vector (not shown).

Game::Game( int _gameType , std::vector< Player > _player )
{
	player = _player;
	state = GAME_STATE_WAITCONNECT;

	std::cout << "-- Game created!\n"
			  << "\t" << player.size() << "\n"
		          << "\t" << player[0].netID << "\n"
			  << "\t" << this << "\n"
			  << "\t" << state << "\n";

	thread = new boost::thread( &Game::main , &this[0] );
}

void Game::main()
{
	std::cout << "Loop of: " << this << "\n"    //Same as constructor//
                  << "\t" << player.size() << "\n"  //ALWAYS ZERO
	          << "\t" << state << "\n";         //Same as constructor//
}

The problem is that state (class variable) gets changed and read properly from both ends, player does NOT. Player, when printed its ".size()" inside the thread, always produces 0, regardless of how many is fed into the constructor.

To reiterate myself:
The constructor outputs everything expected, whereas in main (called when thread starts) player (a vector) is always empty.

Does anyone see/know what the problem is here?

[rollie]

My assumption is that the Game object is being destructed before main(). Define a destructor, output something there, and see if that's the case. Can you show us the code where Game objects are being created and loaded into your vector? Also of note - vectors often have to re-allocate storage, if they grow beyond their bounds, which is going to invalidate the 'this' pointers you are storing when creating new boost::threads. Change your containing vector from vector<Game> to vector<Game *>, and load it via myVector.push_back(new Game(...));

[Disch]

I don't think reallocating will be a problem here because he isn't referring to a vector via a pointer to its elements, he's referring to it through to the object itself... so there's no way it could be out of sync (provided there aren't any issues with multiple threads accessing it at once, but I don't see any indication of that here).

@OP:

Is this code the actual code that produces the problem? Or is this a simplified version? If it's simplified, did you try actually running it and making sure the problems still exists?

The reason I ask is that I don't see any reason why this code wouldn't work as posted. So I really think you are doing something else wrong that isn't posted here.


EDIT:

I just tried out your minimal example and it works without a problem:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47

#include <boost/thread.hpp> #include <vector> class Player { private: int whatever[60]; }; class Game { public: Game(int _gameType, std::vector<Player> _player) { player = _player; state = 100; std::cout << "-- Game created!\n" << "\t" << player.size() << "\n" << "\t" << this << "\n" << "\t" << state << "\n"; thread = new boost::thread( &Game::main, this); } void main() { std::cout << "Loop of: " << this << "\n" << "\t" << player.size() << "\n" << "\t" << state << "\n"; } private: int state; std::vector<Player> player; boost::thread* thread; }; int main() { std::vector<Player> p; p.resize(10); Game g(0,p); std::cin.get(); }

-- Game created! 10 004BFDB0 100 Loop of: 004BFDB0 10 100

So the problem must be elsewhere. Can you try coming up with a minimal example that reproduces the problem?

[rollie]

@Disch I changed your example a bit to illustrate what I was thinking.

Code:

class Game
{
public:
	Game() { state = 50; mId = -1;}
	~Game() { state = 25; mId = -2; }
    Game(int id, int _gameType, std::vector<Player> _player)
    {
		mId = id;
        player = _player;
        state = 100;

        std::cout << mId << ") -- Game created!\n"
            << "\t" << player.size() << "\n"
            << "\t" << this << "\n"
            << "\t" << state << "\n";

        thread = new boost::thread( &Game::main, this);
    }

    void main()
    {
        std::cout << mId << ") Loop of: " << this << "\n"
            << "\t" << player.size() << "\n"
            << "\t" << state << "\n";
    }

private:
	int mId;
    int state;
    std::vector<Player> player;
    boost::thread* thread;
};

int main()
{	
    std::vector<Player> p;
	p.resize(10);
	
	std::vector<Game> vecGames;
	vecGames.resize(2);
	for (int i = 1; i < 6; i++)
	{
		vecGames.push_back(Game(i, 0, p));
	}
    

    std::cin.get();
}

Output:

C:\MinGW\Sandbox>a
1) -- Game created!
        10
        0x22ff18
        100
2) -- Game created!
2       ) Loop of: 100x22ff18

                0x22ff180xa

                100100

3) -- Game created!
3       ) Loop of: 100x22ff18

                0x22ff180xa

                100100

34) Loop of: ) -- Game created!
0x22ff18
10
10
0x22ff18
100
100
55) -- Game created!
) Loop of:      0x22ff180xa

                100x22ff18

                100100

-2) Loop of: 0x22ff18
        10
        25

Note the last section of output - mId == -2! This means the destructor was called on the object, but it's still executing. What I suggested may be happening is, OP has a vector size=3, then goes to add a 4th item. The vector needs to resize, so in order to do so, it calls the copy constructor for each element to move it into the new storage. But the original storage (now deleted) is still there, and available for re-allocation. However, the boost thread is still running, and has a pointer to the de-allocated object. When the object's main() is called, it is still in the same location, so the object outputs the same 'this' pointer.

[Disch]

Well yeah I see the problem there, but he didn't say he had a vector<Game> anywhere. I think it stands to reason that he only has one Game object.

[rollie]

You may be right...I took "Its the constructor for the class that is being put in a vector (not shown)." to mean that he was doing something along these lines

[Qqite]

Yep, changing the vectors to std::vector< Game* > fixed the problem. I think it was a problem is vector reallocating for resizing.

This is good that I found this problem this early, because I plan on there being many games being created and ending constantly, so this method will make that safe.

Also, what is a good way to end a thread from outside of the thread? My idea that I'm trying right now is having a boolean instance variable for when if its flipped (from the outside) that the thread will check every once in a while for if it needs to end. Is this the best way?

[helios]

My idea that I'm trying right now is having a boolean instance variable for when if its flipped (from the outside) that the thread will check every once in a while for if it needs to end. Is this the best way?

Most solutions are variations of this with varying degrees of sophistication.
A better solution for your particular case might be to give each thread a (thread-safe) message queue, and have the thread respond to certain messages in specific ways. For example, it will respond to the message QUIT by cleanly terminating itself.

[Qqite]

Arg! It seems that calling game.erase( game.begin() + 0 ) to delete one of the objects is NOT calling the destructor of that game. Is this true that if you have a vector of pointers that it will not delete the object at the pointer that you call to erase?

[helios]

Is this true that if you have a vector of pointers that it will not delete the object at the pointer that you call to erase?

Yes, you have to delete the pointer before removing it from the container.

delete game.front();
game.erase(game.begin());

By the way, if you're going to be removing objects from the front very often, it may be better to use a list (if you don't need random access) or a deque (if you do).
Yet another possibility, that may or may not be a good idea in your particular case, could be to just leave the vector unshrunken, but zero out deleted pointers so they can be reused later on.

[Qqite]

Aha! Thank you so much for your help! You all have helped a lot.