Pointers and crashes

Pointers and crashes

Postby MCMic » 02 Jun 2014, 22:38

Hello

We got several cases of crash in OpenDungeons, sometimes when putting quarter tiles, when a cannon fires, …
I think most of those come from wrongly handled life cycles.
Also, maybe we should think about replacing bare pointers with the appropriate smart pointers from c++11. ( https://en.wikipedia.org/wiki/Smart_poi ... t_pointers )
We can also use const refs as args of some functions.
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 02 Jun 2014, 23:32

Hi,

I agree about using references in parameters when possible.

Also, maybe we should think about replacing bare pointers with the appropriate smart pointers from c++11.

But the evil Keeper pities us, smart pointers aren't an answer to wrong life cycle handling, only a way to hide problems further in this case.
Please, do not go into this.

Regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 08:24

But using bare pointers is kind of deprecated from good coding pratices.
(Also I saw weird methods “delete yourself”, shouldn’t that be in the destructor?)
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 03 Jun 2014, 09:37

Hi :)

As for the DeleteYourself() methods. Yeah, I used to think the same too (putting the code in the destructor.)
But it will trigger crashes in certain cases because of the render request processed after the object deletion.

As far as I understood this mechanism, the dev who created this method did it to make sure the object deletion append at the end of the render request processing queue.
If you see things that proves it wrong or see a better way of doing this, please tell me though.

But using bare pointers is kind of deprecated from good coding practices.

Eh eh. AFAIK, this statement has got roots in the c/c++/c++11 coding evolution never-ending debate, mixed with some javaism. ;)
Be sure I don't want to fight anybody in this subject as it is a waste of time. We both have our opinion on this and I respect that.

What I can't let go is that one should keep in mind that making changes to the code should be done for the better, not just because it is kinda what's done by others.
This weird sentence o' mine means that I have nothing against smart pointers themselves (they're just yet another tool), but I'm not fond you claiming we should use them because it might fix something.

Let's debug the object life-cycle first. If it happens to be useful to use smart pointers to do something, we'll talk about it at that time, right?.

Regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 10:06

Bertram {l Wrote}:Hi :)

As for the DeleteYourself() methods. Yeah, I used to think the same too (putting the code in the destructor.)
But it will trigger crashes in certain cases because of the render request processed after the object deletion.

As far as I understood this mechanism, the dev who created this method did it to make sure the object deletion append at the end of the render request processing queue.
If you see things that proves it wrong or see a better way of doing this, please tell me though.


Using a smart pointer in this case, you can stop referencing the object after adding the last render request, so that the rendermanager is the last one with a pointer to the object. Once it itself stop using it, it will be deleted.

I just think life cycles are easier to handle with this kind of tools, but I won’t force you and I won’t try that big a change my self so I’ll stop arguing.

Regarding crashes, I get one when putting quarter tiles next to existing quarter tiles and there are creatures on them, something like that. The building with CMAKE_BUILD_TYPE=Debug is not working so it’s hard to get a trace.
I also get a crash when cannon fires, while this worked before. (I’d say the crash comes from the MissileObject somehow)

Also, Ogre issue a lot of warnings about needing to re-export meshes or something like that, could someone take care of this?
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 03 Jun 2014, 10:17

Hi,

Using a smart pointer in this case, you can stop referencing the object after adding the last render request, so that the rendermanager is the last one with a pointer to the object. Once it itself stop using it, it will be deleted.

In that specific case, I believe you're right, though. Be sure to document what and why in the code if you happen to do the change.
I just think life cycles are easier to handle with this kind of tools, but I won’t force you and I won’t try that big a change my self so I’ll stop arguing.

Np. I'm happy we can freely talk about this without the usual ranting. ;)

Regarding crashes, I get one when putting quarter tiles next to existing quarter tiles and there are creatures on them, something like that. The building with CMAKE_BUILD_TYPE=Debug is not working so it’s hard to get a trace.

I believe you should ask hwoarangmy. He has become an expert in this area. I'll also add this in the deocumentation:
http://www.ogre3d.org/forums/viewtopic. ... 05#p384705

I also get a crash when cannon fires, while this worked before. (I’d say the crash comes from the MissileObject somehow)

I added that in the todo-list some time ago IIRC.

Also, Ogre issue a lot of warnings about needing to re-export meshes or something like that, could someone take care of this?

Yep, this is in the todo-list. easy things nobody cares for till now.
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 11:36

I had to do the following changes to get the debug build type to build: http://pastebin.com/jThLnhnx
But it still fails to launch saying it could not find RenderSystem_GL_d.so (and indeed I don’t have the debug version for this one)

Is it possible to add debug symbols for our code but still use the nondebug OGRE libs?
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 03 Jun 2014, 12:43

Hi MCMic,

{l Code}: {l Select All Code}
-    #${CEGUI_LIBRARIES_DEBUG}
+    ${CEGUI_LIBRARIES_DEBUG}

N.B.: This change is personal and you'll find it only in my repo. this is due to my linux dev environment being a bit twisted, eh. ;)
You can remove this by reverting this commit:
https://github.com/Bertram25/OpenDungeo ... 5e97463cc5

Is it possible to add debug symbols for our code but still use the nondebug OGRE libs?

I guess so. Never tried though. May be feasible by passing a few flags when invoking cmake?
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 12:54

I managed to get it working modifying plugins_d.cfg.

So here is the trace of the crash: http://pastebin.com/W4BczVuC
Trying to print animatedObjects, I get an error about memory at address 0x0…

So I’m not even sure the trace will help as it seems something else messed with the memory, otherwise I don’t see how the vector could be corrupted.
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 03 Jun 2014, 13:09

Hi,

Here, maybe the invocation of the animated object makes the game crash.

Acording to the line 365 in gamemap.cpp:
{l Code}: {l Select All Code}
if (a == animatedObjects[i])

Either 'a' or 'animatedObjects[i]' is triggering the crash, I suppose.
You could first try to cout a and the latter values in different lines to see which one is actually wrong.
you could also make sure not to add NULL objects by adding a check in:
void GameMap::addAnimatedObject(MovableGameEntity *a)


My two cents.

Regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 16:05

I already used gdb and saw that the problem is that animatedObjects is messed up. (size is like too big and memory address is supposed to be 0x0)
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 03 Jun 2014, 16:23

Ok, well, I'll check more thoroughly if you don't find it before that.
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: Pointers and crashes

Postby hwoarangmy » 03 Jun 2014, 18:07

I confirm that debug version should work as I almost use it only :)
Concerning the "delete yourself" thing, I have seen stuff like this in Qt libraries where there is a function deleteLater() that allows to delete the object when its state allows it. I guess the function you are talking about does the same.

Concerning the use of smart pointers, I also believe it would hide some architecture problems that would be even more difficult to debug when they will happen randomly. And I've read some debates between pros/cons smart pointers and the summary usually is that smart pointers are more expensive that normal pointers (since the system has to handle the number of pointers for each instance) so they should be used only when needed.
hwoarangmy
 
Posts: 567
Joined: 16 Apr 2014, 19:13

Re: Pointers and crashes

Postby MCMic » 03 Jun 2014, 18:28

Why is the deleteTile handling commented out in RenderManager?
Why is deleteRoomObject not handled?

Why is tile trigerring both deleteTile and destroyTile in deleteYourself while destroyMesh should have thigerred destroyTile already as for others (this is in GameEntity.cpp) ?
User avatar
MCMic
 
Posts: 723
Joined: 05 Jan 2010, 17:40

Re: Pointers and crashes

Postby Bertram » 04 Jun 2014, 08:46

Hi MCMic,

That's relevant questions. :)
I do think the handling of certain objects has been tinkered until it was working, but I also saw several inconsistencies throughout the code about that. (I already fixed several, or hopefully ;])

AFAIK, the actual Room Object mesh deletion is done here:
https://github.com/Bertram25/OpenDungeo ... m.cpp#L299
https://github.com/Bertram25/OpenDungeo ... y.cpp#L178
https://github.com/Bertram25/OpenDungeo ... r.cpp#L795
but it seems you catched something since I didn't see any room object deletion in the room code.
Just note that when a room is absorbing another one, the room object aren't destroyed right away, there are transfered to the new room.

Why is tile trigerring both deleteTile and destroyTile in deleteYourself while destroyMesh should have thigerred destroyTile already as for others (this is in GameEntity.cpp) ?

You're right. this is looking inconsistent. I never got around fixing this. Does it crash if you make destroyMesh() trigger destroyTile()?

Regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Who is online

Users browsing this forum: No registered users and 1 guest