Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » Timers + smart pointers + LLVM vs. G++

This thread is locked; no one can reply to it. rss feed Print
Timers + smart pointers + LLVM vs. G++
Artelis
Member #10,862
April 2009

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:

#SelectExpand
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
Member #476
June 2000
avatar

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.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Artelis
Member #10,862
April 2009

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
Member #476
June 2000
avatar

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.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

Artelis
Member #10,862
April 2009

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

#SelectExpand
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?

bamccaig
Member #7,536
July 2006
avatar

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.

taron 
Member #10,584
January 2009
avatar

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.

Artelis
Member #10,862
April 2009

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

This must have been giving us undefined behavior:

#SelectExpand
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).

#SelectExpand
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.

pkrcel
Member #14,001
February 2012

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.

It is unlikely that Google shares your distaste for capitalism. - Derezo
If one had the eternity of time, one would do things later. - Johan Halmén

Thomas Fjellstrom
Member #476
June 2000
avatar

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.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

beoran
Member #12,636
March 2011

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! :)

pkrcel
Member #14,001
February 2012

I'd say it's specified behavior.

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

It is unlikely that Google shares your distaste for capitalism. - Derezo
If one had the eternity of time, one would do things later. - Johan Halmén

bamccaig
Member #7,536
July 2006
avatar

Go to: