Brainstorming: refactoring classes/inheritances

[svenskmand]

Then getting the debugger to run should probably have a higher priority than the bug, as it is almost impossible and very time consuming to debug code without a debugger :S

[StefanP.MUC]

The problem here is that I don't have time to dig deeply into hardcore debugging matters (which are not very easy). Programming is just my hobby and I originally started with C#, where most things are much easier than in C++.

[oln]

I am going to try to debug myself, I thought my windows VM had crashed, but apparently not.

EDIT: Seems like it wasn't working properly after all.

[StefanP.MUC]

OK, a new result:

GameMap.cpp, line 840 (GameMap::doTurn) is where the crash happens:
pthread_join(thread1, NULL);

Up until this point everything seems to get correctly executed. This is the first line that doesn't pop up in the log if I spam the thread relevant part with LogManager output.

So it seems to be related to GameMap::doCreatureTurns which is the only relevant function call in there.

edit: hmm, nope, not neccessarily... Now I got to round two when teh bug happened... This is driving me crazy...

[oln]

line 340 of GameMap.cpp is GameMap::circularRegion from what I can see. Is that where it crashes? If so, I did do a small change that moved the locks (http://opendungeons.git.sourceforge.net ... d117935326) a while back, but I can't see how that should cause crashes.

Anyhow, it does make sense that the crash happens in doCreatureTurns somewhere, as that is only run when in game mode and not in the editor.

[StefanP.MUC]

Sorry, I meant line 840... But this seems not neccessarily the case, because in very rare cases the game manages to perform this line and crash very shortly after (depending on how long the threads took).
But I think it has to be either in GameMap::creatureDoTurnThread or in GameMap::miscUpkeepThread.
But I'm not able to find out where...

[oln]

I would guess that it could be related to the render requests, as the creature object sends a pointer to itself to the rendermanager, which isn't ideal. I got crashes there after we changed the inheritance thing, but I fixed the ones I got by sending copies of the relevant data instead of the creature object itself (which seems to only be the name).
I can see that Creature::getName is called from both threads, and it's not being locked. Maybe this could be an issue. So either we have to make a lock/semaphore, or alter the code so it's only used by one of the threads.

[StefanP.MUC]

Yes, this sounds like a good plan. I think introducing a semaphore is the easier way here.

[oln]

Yeah, it's the easiest way to check if that's the issue.

[StefanP.MUC]

Hmm, putting a semaphore in getName() let the game already crash when entering the main menu....

[oln]

That's strange. Did you remember to initialise the semaphore?

[StefanP.MUC]

Yes. I just figured out that the crash happens not on the sem_wait/post, but even on sem_init itself... This is what I added to GameEntity (and this already leads to a crash):

{l Code}: {l Select All Code}

//include this header
#include <semaphore.h>

class GameEntity
{
  public:
    GameEntity()
    {
      //init semaphore, init with 1, 1 also crashes
      sem_init(&nameLockSemaphore, 0, 1);
    }

  private:
    //add this variable
    mutable sem_t nameLockSemaphore;
};


[oln]

That sounds really strange. I can't see any reason that sem_init should crash other than there GameEntity object not being initialised. (Though then there should be more crashes)

I tried to compile and run again myself. (I have reinstalled my OS.) I got an assertion fail in boost::threads deep inside some cegui code, though it might be related to me using ogre from the official repos and cegui from the repository we used previously.

[StefanP.MUC]

I'm not sure what else I could try to get this test for the thread bug running. If not even the semaphore can be initialised I can't run further tests...

[svenskmand]

Maybe now is the time to completely ditch pthreads library and use the boost library? That way the existing semaphore code will also be checked, which might be a good idea since it seems to cause quite some problems.

[oln]

The whole thread model needs to be refactored really, right now there are locks everywhere. I am not sure what the best option is though, either way that will be a lot of work.
I am worried that this is actually a bug that has been there before we got the crashes, but has surfaced after doing the changes.
I was going to try to do some tests with valgrind, but I got some issues with building because the CEGUI and Ogre packages didn't match.

What was the last release where you didn't get the crash?

[svenskmand]

The msi package on sf works But I do not know what revision that is I should probably put the GIT and SVN revision number into the builds I make when running my scripts :S

[oln]

It's from later than the last release, it's in one of the like 20 last commits I think. I think it was working right before we started making the gameEntity class.

[StefanP.MUC]

Yes, If I remember correctly the crash popped up with either the GameEntity class or with the refactoring regarding CreatureDefinition (I'd guess rather this last one). Problem there was, that this refactoring couldn't be done in small steps, because everything was cross-inheriting and -referencing instead of having a straight top-to-bottom dependence that we were intending to create with the refactoring.

[aspidites]

The msi package on sf works But I do not know what revision that is I should probably put the GIT and SVN revision number into the builds I make when running my scripts :S

Well, the git branch did get tagged at revision 50404612d421e26dd6b33ca2af9f5e496f0b25c5 before making this package, which was at the end of April, if that helps. Unfortunately, there's been 50+ commits since then.

[oln]

For some reason CEGUI::OgreRenderer::bootstrapSystem() fails when running in debug mode.

New version of CEGUI is out, might be worth a try.
http://www.cegui.org.uk/phpBB2/viewtopic.php?f=6&t=5978

[svenskmand]

Nice If we could get the killer bug squashed we can move this boat forward

[oln]

I'm getting seemingly random crashes shortly after clicking new game now, it's possible that this is the same issue as what happens on windows.

[svenskmand]

That bug is really persistent :S

[StefanP.MUC]

The Tile class has everything from GameEntity. Would it make sense to make Tile also inherit from GameEntity? Not sure if this is logic or not (because Tiles are no real object that the player has to interfere with, like Rooms, Traps and Creatures).

There are also a lot of other annoying things. GameEntity stores the level, but AttackableEntity also requires a getLevel() method. Just removing it from AttackableEntity does not work, because there's code calling it... And casting everything to anything is a bug-catcher...