Threading issues...

[10oTURfi]

So I have this piece of code:

#include "Game.hpp"

// ctor/dtor
// ...

void Game::Run()
{
    sf::Event Event;

    while(Window->isOpen())
    {
        StateMutex.lock();
        if(NewState)
        {
            delete CurrentState;
            this->CurrentState = NewState;
            NewState = nullptr;
        }
        StateMutex.unlock();
        while(Window->pollEvent(Event))
        {
            CurrentState->HandleEvent(Event);
        }
        Window->clear();
        CurrentState->Draw();
        Window->display();
    }
}

void Game::ChangeState(GameState* NewState)
{
    boost::mutex::scoped_lock(StateMutex);
    this->NewState = NewState;
}

Game::Run is loop of main thread, while ChangeState is (when necessary) called by other, boost::thread. When this program crashes, the boost::thread is at line this->NewState = NewState;, while main thread is sleeping (Window->display())

If I switch Thread in debugger to the thread calling the ChangeState, I notice that this pointer is invalid (0xcdcdcdcd)... and thats why I get access violation error, but if I switch back to main thread, the this pointer is stil valid (pointing somewhere randomly)...

That thread called Game::ChangeState from this code:

void WorldSession::HandleLoginOpcode()
{
    // bla bla ....
    World* pWorld = new World(MeID);
    this->pWorld = pWorld;
    pWorld->LoadTileMap(MapID);
    sGame->ChangeState(pWorld);
    printf("Packet is good!\n");
}

And the sGame pointer here is still valid pointer...

Why does that happen?

[Ivan Sidarau]

This line
boost::mutex::scoped_lock(StateMutex);
should be fixed to
boost::mutex::scoped_lock lock_for_change_state(StateMutex);

otherwise it unlocks at the same line.

[10oTURfi]

Now crashes on that very line.

[Ivan Sidarau]

show full source code for Game.hpp Game.cpp

[10oTURfi]

Game.hpp

#ifndef GAME_H
#define GAME_H

#include "GameState.hpp"
#include "Globals.hpp"
#include "boost/thread.hpp"

class Game
{
public:
    Game(bool FullScreen);
    ~Game();
    void Run();

    void ChangeState(GameState* NewState);

private:
    GameState* CurrentState;
    boost::mutex StateMutex;
    GameState* NewState;
};

#endif 

Game.cpp

#include "Game.hpp"

Game::Game  (bool FullScreen) :
CurrentState(nullptr),
NewState    (nullptr)
{
    Window->create(sf::VideoMode(WindowWidth, WindowHeight), "Warrior of Dreamworld", FullScreen ? sf::Style::Fullscreen : sf::Style::Close);
    Window->setFramerateLimit(60);
}

Game::~Game()
{
    delete CurrentState;
}

void Game::Run()
{
    sf::Event Event;

    while(Window->isOpen())
    {
        StateMutex.lock();
        if(NewState)
        {
            delete CurrentState;
            this->CurrentState = NewState;
            NewState = nullptr;
        }
        StateMutex.unlock();
        while(Window->pollEvent(Event))
        {
            CurrentState->HandleEvent(Event);
        }
        Window->clear();
        CurrentState->Draw();
        Window->display();
    }
}

void Game::ChangeState(GameState* NewState)
{
    boost::mutex::scoped_lock lock_for_change_state(StateMutex);
    this->NewState = NewState;
}

[coder777]

The problem seems more on the sGame side. It crashes because that pointer is invalid (even if it looks valid)

[10oTURfi]

sGame MUST be valid.

// Somewhere in main.cpp
sGame = new Game(true);

Its not deleted at all (Oh my, I just found a mem leak... Tho it doesnt matter cause Game object must live untill program ends...)

[Ivan Sidarau]

I don't see anything criminal at this code, maybe main.cpp have problems.

[10oTURfi]

Main.cpp

#include "Login.hpp"
#include "Game.hpp"
#include "WorldSession.hpp"
#include "boost/scoped_ptr.hpp"

int main()
{
    using namespace std;

    ofstream ErrorLog("Error Log.txt");
    cerr.rdbuf(ErrorLog.rdbuf());

    try
    {
        cerr << "Guessing screen resolution [FIXME]: Select configuration in game and save it in Config.conf" << endl;
        WindowWidth = (*sf::VideoMode::getFullscreenModes().begin()).width;
        WindowHeight = (*sf::VideoMode::getFullscreenModes().begin()).height;
        cerr << "My guess is: " << WindowWidth << "x" << WindowHeight << endl;

        boost::asio::io_service io;
        tcp::resolver Resolver(io);
        tcp::resolver::query Query("127.0.0.1", "48879");
        tcp::resolver::iterator Iterator = Resolver.resolve(Query);

        sGame = new Game(true);

        Session = new WorldSession(io, Iterator);
        {// TODO: this code is useless... See above
            ifstream ConfigFile("Config.conf");

            if(!ConfigFile)
                throw std::exception("Cannot open Config.conf");

            std::string Ip;
            ConfigFile >> Ip;

            sGame->ChangeState(new Login());
        }
        boost::thread NetworkThread(boost::bind(&boost::asio::io_service::run, &io));
        sGame->Run();
    }
    catch(std::exception& e)
    {
        cerr << e.what();
    }
    catch(...)
    {
        cerr << "Unhandled exception";
    }

    delete Session;
    delete Window;
    delete sGame;

    return 0;
}

[Ivan Sidarau]

sGame->ChangeState(new Login()); <- do you meet error here ?

where declaration of "sGame" placed?

[10oTURfi]

No error there. Only when I change state from Login to World (see first post).

sGame is declared in Globals.hpp

Globals.hpp

#ifndef GLOBALS_H
#define GLOBALS_H

#include "../shared/Defines.hpp"
extern uint16 WindowWidth;
extern uint16 WindowHeight;
extern sf::RenderWindow* Window;

#include <iostream>
#include <fstream>

#include "WorldSession.hpp"
class WorldSession;
extern WorldSession* Session;

#include "Game.hpp"
class Game;
extern Game* sGame;

#endif 

Globals.cpp

#include "Globals.hpp"

sf::RenderWindow* Window;
WorldSession* Session;
Game* sGame;
uint16 WindowWidth;
uint16 WindowHeight;

[Ivan Sidarau]

Hm, nothing bad here...

Try to use boost::singleton for sGame.

What compiler do you use?

Please add
pWorld->LoadTileMap(MapID);
std::cout << sGame << std::endl; // new line
sGame->ChangeState(pWorld);

If it invalid, possible that your program try to access to deleted pointer when exit on some exception.
Or possible that you have different sGame on WorldSession.cpp, main.cpp (may be dll-exe linking)...

[10oTURfi]

There is no such thing as boost::singleton (at least in my version). There are few in /detail/ but seem to be only for internal usage.

I use MSVC++ 2010.

That outputs CDCDCDCD.

There is no dll linking, WorldSession is in exe... Nor it was possible that it got deleted, as I said, main thread is usually sleeping in Window->display(); (which is actually where it spends 99.99% of time) when it crashes

[Ivan Sidarau]

You could send me source code if you like to.
ivan.sidarau (at) gmail.com

I don't see any problems in here.

[10oTURfi]

Even better, you could git clone it yourself: (or download zipball)
https://github.com/krofna/Warrior-of-Dreamworld
(NOTE: branch Thread is the bugged one)

[ne555]

You've got a weird include mechanism.
¿Have you got a makefile?

[10oTURfi]

Nop. No make files.

What is so weird about my include mechanism?

[viliml]

The weird thing about your include mechanism is: YOU'VE GOT INCLUDES ALL OVER THE PLACE!!!

[10oTURfi]

Oh... that... I suppose it can't be avoided. Apart from damn slow compile time, there really are no real side effects :)

[ne555]

Game.cpp:31:12: warning: deleting object of abstract class type ‘GameState’ which has non-virtual destructor will cause undefined behaviour
World.cpp:43:25: warning: deleting object of polymorphic class type ‘WorldObject’ which has non-virtual destructor might cause undefined behaviour

Get used to compile with warnings.

About the includes, some are relatives, other suppose that you include the path in the command.

¿Which version of SFML are you using? By instance, Login fails because there is no sf::Keyboard::Backspace (in 2.0 there is Back, in 1.6 there is no sf::Keyboard)

Done compiling, ¿what do I link against?
talked too soon, damn sql