Timers + smart pointers + LLVM vs. G++

This one is a real doozy.

Here is the background. My collaborator is on Linux using GCC G++ as his compiler with C++11. I'm on OS X with Xcode 5, so my compiler is LLVM (which I understand to be clang++).

I have a vector of smart pointers to an object called GenericEnemy:

vector<unique_ptr<GenericEnemy> > enemies_;

Then during a game logic tick I iterate through each GenericEnemy, send a tick to it which checks if changeDirectionTimer_ has ticked. If so, select a new, random direction for the enemy:

1ALLEGRO_EVENT e; 2al_get_next_event(changeDirectionQueue_, &e); 3if(e.type == ALLEGRO_EVENT_TIMER) 4 movingDirection_ = (kMoveDirection)(rand() % (int)kMoveDirectionCount);

SO, this all works fine and dandy on his computer. On mine, the timer ticks once as it should (a few seconds), and then suddenly it ticks every logic tick, as if e.type == ALLEGRO_EVENT_TIMER is ALWAYS true. I've even tried flushing the queue after every movingDirection_ update.

HERE is the really weird part. If I remove the use of smart pointers and do this:

vector<GenericEnemy*> enemies_;

Then everything works as it should on both OS's. Does anyone have ANY clue what could be causing this? We would realllllllly like to use smart pointers in this project.

Thomas Fjellstrom

Are you sure you want a unique_ptr? I'm pretty sure the very first instance of the unique_ptr will delete your object as soon as it goes away. Any copy of that unique_ptr will also want to delete the underlying object.


The pointer would only be deleted if I removed it from it's vector, right? I'm checking, and GenericEnemy::~GenericEnemy() is never called.

Update - now it's happening on Linux, too. We didn't even change any code... ???

Thomas Fjellstrom

Hm, well if you ever made a copy of the unique_ptr, that copy would then want to delete your object iirc. Oh, and when resizing, the vector can and will copy the items its storing, which will copy the unique_ptr... I am not 100% sure about unique_ptr always deleting its object the first time the unique_ptr's dtor is called, but I think its a safe bet.

Random errors tend to mean memory corruption, or double free's. So you may want to look for memory errors.

Try running under valgrind on linux. see if it tells you about any issues.


Sorry for confusion. We're both new to smart pointers. Here is the interaction with the unique_ptr's

1SquareView* p_view = player_->get_view(); 2vector<unique_ptr<GenericEnemy> >::iterator i = enemies_.begin(); 3 4while(i != enemies_.end()) { 5 GenericEnemy* enemy = (*i).get(); 6 enemy->update(); //This is where the timer is ticking 7 8 SquareView* e_view = enemy->get_view(); 9 10 //box collision detection 11 int bottom1 = p_view->y_ + p_view->height_, 12 top1 = p_view->y_, 13 left1 = p_view->x_, 14 right1 = p_view->x_ + p_view->width_; 15 16 int bottom2 = e_view->y_ + e_view->height_, 17 top2 = e_view->y_, 18 left2 = e_view->x_, 19 right2 = e_view->x_ + e_view->width_; 20 21 if( !(bottom1 < top2 || top1 > bottom2 || left1 > right2 || right1 < left2) ) { 22 enemy->make_dead(); 23 } 24 25 if(enemy->is_alive()) { 26 ++i; 27 } 28 else { 29 i = enemies_.erase(i); 30 } 31}

So the vector only resizes if any enemy is hit. The GenericEnemy::~GenericEnemy() is definitely called when it's hit and we get:

i = enemies_.erase(i); //vector resize causes the unique_ptr to delete.

Do you see anything in this bit of code that would create issues?


My first hunch was the incorrect smart pointer type as well. I imagine you want a std::shared_ptr<T> (C++11) or boost::shared_ptr<T> (or maybe a std::auto_ptr<T>, but IIRC these suck for reasons that I don't know about).

Building on what Thomas Fjellstrom said, I don't even think that std::unique_ptr can be copied, and if that's correct then it isn't even compatible with most (any?) STL containers, such as std::vector.

As a rule when using smart pointers you probably want shared_ptr<T>, but it's important to be aware of the semantics of each because no single object is right for all use cases.


He isn't using the wrong pointer type.
It's completely legal to have a raw pointer point to the same address as a unique_ptr, an unique_ptr implies unique ownership; raw pointers are still useful as weak references. unique_ptr is also very much compatible with the STL as opposed to auto_ptr, which is one of the biggest reasons auto_ptr sucks.

If you're creating unique_ptrs and then inserting them into a vector, I'm pretty sure you must use the emplace_back method to avoid copies, if you're inserting raw pointers make sure they point to unique addresses on the heap.
I personally just push_back raw pointers.

Also, the auto keyword makes your life easier.

vector<unique_ptr<GenericEnemy> >::iterator i = enemies_.begin();  
// Can be rewritten as
auto i = enemies_.begin();

As to why your program has this weird behaviour, I have no idea.


Thanks for the tips, everyone. We figured out the odd behavior.

This must have been giving us undefined behavior:

1al_get_next_event(changeDirectionQueue_, &e)) 2if(e.type == ALLEGRO_EVENT_TIMER) 3 //do something

Oddly enough after the first tick, e.type was always ALLEGRO_EVENT_TIMER, even though there was no event on the queue. SO, we just encapsulated the get next event in an if (al_get_next_event returns false if it didn't find an event).

1if(al_get_next_event(changeDirectionQueue_, &e)) { 2 if(e.type == ALLEGRO_EVENT_TIMER) { 3 //do something 4 } 5}

No more undefined behavior - we only get to the event type checking iff we find an event. I assume we were still getting undefined behavior without using smart pointers, but it was just a more favorable undefined behavior.


Going with the manual for al_get_next_event, I'd say you're getting unspecified behaviour.

And most prolly (haven't checked impelmentation) the referenced returning event is left unchanged if there si no event.

Thomas Fjellstrom

I'd say it's specified behavior. The docs state that the contents of the event struct are unspecified, aka: do not expect the contents to be valid in any way if the function returns false.


Indeed, looking at event.c, it turns out that al_get_next_event will leave the data in the event struct unchanged if there is no next event. Which explains exactly why the irratic bhaviour was seen, as the struct was no initialiazed to anuthing really. Morale of the story: always check the return values! :)


I'd say it's specified behavior.

Fair enuff, I guess I worded my post poorly :P

beoran said:

Morale of the story: always check the return values! :)

Well said.

Thread #614043. Printed from Allegro.cc