Client managing

[PhiLLe]

Hi!

I am writing a little Server application.
My Problem is that the Socket for my client disconnects when I am storing my Socket in a vector and I push back an other object.

Code:

Code: [Select]

vector<Player> Clients;

void Accept(void * UserData)
{
    sf::SocketTCP Listener;
    Player NewClient;

    if (!Listener.Listen(PORT))
        cerr << "Clouldn't Bind to port: " << PORT << endl;

    while (true)
    {
        Clients.push_back(NewClient);
        Listener.Accept(Clients.back().Socket);
   
        cout << "New connection. Address: "  << endl;
    }
   
    Listener.Close();
}

My player class:

Code: [Select]

class Player {
    public:
       
        ~Player()
        {
            Socket.Close();
        }

        sf::SocketTCP Socket;
        sf::IPAddress Address;

    private:

};

So the problem occurs when "Clients.push_back(NewClient);" is called the second time. I don't know why but the Socket "Clients.at(0).Socket" just disconnects.

I hope somebody can help me.

[Laurent]

How do you know that the socket is disconnected? Do you store pointers or references to the elements of your array?

[PhiLLe]

How do you know that the socket is disconnected? Do you store pointers or references to the elements of your array?

I know it is disconnected because the Client says it is when it tries to receive anything and the server confirms that when I try to send anything.

I do not store pointers. I create a Player object at the beginning and push it back each time I need a new object. After that I fill the element with the socket: "Listener.Accept(Clients.back().Socket); "

[Laurent]

It's hard to tell without seeing the rest of the code.

Maybe you should try to provide a complete and minimal example that reproduces your problem.

[PhiLLe]

main.cpp

Code: [Select]

#include <iostream>
#include <SFML/Network.hpp>
#include "player.h"
using namespace std;
using namespace sf;

vector<Player> Clients;

void Accept(void * UserData)
{
    sf::SocketTCP Listener;
    Player NewClient;

    if (!Listener.Listen(1234))
        cerr << "Clouldn't Bind to port: " << PORT << endl;

    while (true)
    {
        Clients.push_back(NewClient);
        Listener.Accept(Clients.back().Socket);
   
        cout << "New connection. Address: "  << endl;
    }
   
    Listener.Close();
}

int main()
{
   
    sf::Thread Taccept(Accept);

    Taccept.Launch();

    while (true)
    {
        sf::Packet packet;
        sf::Int8 GameChar = 'P';
       
        packet << GameChar;

        for (unsigned int i = 0; i < Clients.size(); i++)
        {
            if (Clients.at(i).Socket.IsValid())
                Clients.at(i).Socket.Send(packet);
        }

        sf::Sleep(1);
    }

    Taccept.Terminate();

    cin.get();
    return 0;
}


player.h

Code: [Select]

#ifndef __PLAYER__
#define __PLAYER__

#include <SFML/Network.hpp>

class Player {
    public:
       
        ~Player()
        {
            Socket.Close();
        }

        sf::SocketTCP Socket;
        sf::IPAddress Address;

    private:

};

#endif

I know my Thread handling is bad. Just for testing purpose right now.
Once again: The first Socket that connects disconnects when "Listener.Accept(Clients.back().Socket);" is called the second time.

Btw: I am trying to make a pong(sfml sample) multiplayer mode

[Laurent]

I can see three major errors in this code.

1. You access the clients vector from both threads, but you don't protect it (with a mutex). Bad things may happen

2. You never check the return value of the socket functions (Accept may return an error)

3. You add a client to the vector before its socket is connected, you should rather do that:

Code: [Select]

while (true)
{
    sf::SocketTCP Socket;
    if (Listener.Accept(Socket) == sf::Socket::Done)
        Clients.push_back(Player(Socket)); // implement the corresponding constructor in Player
}

[PhiLLe]

Thank you for the help so far. But there is still a problem.

main.cpp

Code: [Select]

#include <iostream>
#include <vector>
#include <SFML/Network.hpp>
#include "player.h"
using namespace std;

vector<Player> Clients;
sf::Mutex GlobalMutex;

void Accept(void * UserData)
{
    sf::SocketTCP Listener;

    if (!Listener.Listen(1234))
        cerr << "Clouldn't Bind to port: " << 1234 << endl;

    while (true)
    {
        sf::SocketTCP Client;
        if (Listener.Accept(Client) == sf::Socket::Done)
        {
            GlobalMutex.Lock();
            Clients.push_back(Player(Client));
            GlobalMutex.Unlock();

            cout << "New connection" << endl;
        }
    }
   
    Listener.Close();
}

int main()
{
    sf::Thread Taccept(Accept);
    Taccept.Launch();

    while (true)
    {
        sf::Packet packet;
        sf::Int8 GameChar = 'P';
        packet << GameChar;
 
        GlobalMutex.Lock();
        for (unsigned int i = 0; i < Clients.size(); i++)
        {
            sf::Socket::Status status = Clients.at(i).Socket.Send(packet);
            cout << "Socket " << i << ": ";
            if (status == sf::Socket::Error)
                cout << "Error" << endl;
            else if (status == sf::Socket::Disconnected)
                cout << "Disconnected" << endl;
            else if (status == sf::Socket::NotReady)
                cout << "NotReady" << endl;
        }
        GlobalMutex.Unlock();
        sf::Sleep(0.5);
    }
    Taccept.Terminate();
    return 0;
}


player.h

Code: [Select]

#ifndef __PLAYER__
#define __PLAYER__

#include <SFML/Network.hpp>

class Player {
    public:
       
        ~Player()
        {
            Socket.Close();
        }

        Player(sf::SocketTCP Client)
        {
            Socket = Client;
        }

        sf::SocketTCP Socket;
};

#endif


main.cpp (simple Client)

Code: [Select]

#include <iostream>
#include <SFML/Network.hpp>
using namespace std;

int main()
{
    sf::SocketTCP Server;
   
    if (Server.Connect(1234, "127.0.0.1") == sf::Socket::Done)
    {
        while(1)
        {
            sf::Packet packet;
            sf::Socket::Status status = Server.Receive(packet);
            if (status == sf::Socket::Done)
            {
                sf::Int8  GameChar;
               
                packet >> GameChar;
                cout << "Message: " << GameChar << endl;
            }
            else if (status == sf::Socket::Disconnected)
            {
                cout << "Disconnected" << endl;
            }
            else if (status == sf::Socket::Error)
            {
                cout << "Error" << endl;
            }
            else if (status == sf::Socket::NotReady)
            {
                cout << "NotReady" << endl;
            }
            sf::Sleep(0.5);
        }
    }
    else
    {
        cout << "Failed to connect" << endl;
    }

    cin.get();

    return 0;
}


Now, the client is disconnected whenever "Clients.push_back(Player(Client));" is called. I really don't know why even after 30 minutes of debugging.
What I know is that the client says it is disconnected. The Server says there is an error when trying to send the packet.
Does it matter if I use pointers or not? Should I use them?

I hope you can help me.

[Laurent]

The problem is that the semantics of your class are not well defined. It is copyable (and it is copied when you add instances to the vector), but the copy operation is not properly implemented: several copies of the same player will share the same socket handle, but whenever one of those players will be destroyed, this handle will be closed and the all other copies will report a disconnection.

So you should ensure that your Player class should never be copied (make it inherit from sf::NonCopyable for example) and store pointers in your vector.

[PhiLLe]

Thanks a lot again.
I tried all the things you said in a lot of different variants. I still couldn't get it to work.

I tried all combinations of:
- storing pointers in my vector
- storing a pointer to the Socket in my class (sf::SocketTCP * Socket;)
- inherit from sf::NonCopyable
- making "sf::SocketTCP * Socket;" private

I don't know what to do.

Is there any other easy way to build a Client Manager except vectors?

[Laurent]

Storing pointers should solve the problem. Can you show your new code?

[PhiLLe]

player.h

Code: [Select]

#ifndef __PLAYER__
#define __PLAYER__

#include <SFML/Network.hpp>

class Player : sf::NonCopyable {
    public:
       
        ~Player()
        {
            Socket.Close();
        }

        Player(sf::SocketTCP Client)
        {
            Socket = Client;
        }

        sf::SocketTCP Socket;
};

#endif


main.cpp

Code: [Select]

#include <iostream>
#include <vector>
#include <SFML/Network.hpp>
#include "player.h"
using namespace std;

std::vector<Player *> Clients;
sf::Mutex GlobalMutex;

void Accept(void * UserData)
{
    sf::SocketTCP Listener;

    if (!Listener.Listen(1234))
        cerr << "Clouldn't Bind to port: " << 1234 << endl;

    while (true)
    {
        sf::SocketTCP Client;
        if (Listener.Accept(Client) == sf::Socket::Done)
        {
            GlobalMutex.Lock();
            Clients.push_back(&Player(Client));
            GlobalMutex.Unlock();

            cout << "New connection" << endl;
        }
    }
   
    Listener.Close();
}

int main()
{
    sf::Thread Taccept(Accept);

    Taccept.Launch();

    while (true)
    {
        sf::Packet packet;
        sf::Int8 GameChar = 'P';
        packet << GameChar;
 
        GlobalMutex.Lock();
        for (unsigned int i = 0; i < Clients.size(); i++)
        {
            sf::Socket::Status status = Clients.at(i)->Socket.Send(packet);
            cout << "Socket " << i << ": ";
            if (status == sf::Socket::Error)
                cout << "Error" << endl;
            else if (status == sf::Socket::Disconnected)
                cout << "Disconnected" << endl;
            else if (status == sf::Socket::NotReady)
                cout << "NotReady" << endl;
        }
        GlobalMutex.Unlock();
        sf::Sleep(0.5);
    }

    Taccept.Terminate();

    return 0;
}


[Laurent]

Clients.push_back(&Player(Client));

Here you store the address of an object that will be destroyed before the next line of code is executed. You can't use instances allocated on the stack, you must create them with new (and destroy them with delete).

[PhiLLe]

Thank you very much, Laurent!
It is working correctly now.

I like your way of helping people.

warm regards,
PhiLLe