|
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: 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
|
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. -- |
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
|
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. -- |
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 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
|
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. -- acc.js | al4anim - Allegro 4 Animation library | Allegro 5 VS/NuGet Guide | Allegro.cc Mockup | Allegro.cc <code> Tag | Allegro 4 Timer Example (w/ Semaphores) | Allegro 5 "Winpkg" (MSVC readme) | Bambot | Blog | C++ STL Container Flowchart | Castopulence Software | Check Return Values | Derail? | Is This A Discussion? Flow Chart | Filesystem Hierarchy Standard | Clean Code Talks - Global State and Singletons | How To Use Header Files | GNU/Linux (Debian, Fedora, Gentoo) | rot (rot13, rot47, rotN) | Streaming |
l j
Member #10,584
January 2009
|
He isn't using the wrong pointer type. 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. 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: 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. |
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 |
Thomas Fjellstrom
Member #476
June 2000
|
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. -- |
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
|
Thomas Fjellstrom said: I'd say it's specified behavior. Fair enuff, I guess I worded my post poorly It is unlikely that Google shares your distaste for capitalism. - Derezo |
bamccaig
Member #7,536
July 2006
|
beoran said: Morale of the story: always check the return values! -- acc.js | al4anim - Allegro 4 Animation library | Allegro 5 VS/NuGet Guide | Allegro.cc Mockup | Allegro.cc <code> Tag | Allegro 4 Timer Example (w/ Semaphores) | Allegro 5 "Winpkg" (MSVC readme) | Bambot | Blog | C++ STL Container Flowchart | Castopulence Software | Check Return Values | Derail? | Is This A Discussion? Flow Chart | Filesystem Hierarchy Standard | Clean Code Talks - Global State and Singletons | How To Use Header Files | GNU/Linux (Debian, Fedora, Gentoo) | rot (rot13, rot47, rotN) | Streaming |
|