Many TCP connections?

[Saitei]

I tried to understand the SFML network and I'm making some progress.
It seems to be good, but there were problems:
1)I do not know how to create a lot of TCP connections in many threads.
2)I do not understand what is sf::SocketSelector (many times re-read the documentation, but could not understand the essence.)

Honestly, I used the search and read other people's posts ... My level of English is not good enough to understand everything 100% ... 

[Laurent]

1)I do not know how to create a lot of TCP connections in many threads.

You'll have to be more specific about what your problem is, not what you think you need to solve it.

2)I do not understand what is sf::SocketSelector

You have, let's say, 2 sockets in blocking mode. And you want to do something like this:

socket1.receive(...);
socket2.receive(...);

The problem is: if some data arrives on socket2 before socket1, your app will be blocked, waiting for socket1 to receive data before socket2 can finally deliver its pending data. sf::SocketSelector solves this problem, by unblocking as soon as one of the sockets receives data.

selector.add(socket1);
selector.add(socket2);

if (selector.wait())
{
    // the selector will tell you which socket has received data, and you can call "receive" on it directly
}

If it's still not clear then play with a server that accepts more than 1 client. You'll understand the problem quickly

[Saitei]

1)I do not know how to create a lot of TCP connections in many threads.

You'll have to be more specific about what your problem is, not what you think you need to solve it.

Okay, I will try to describe the problem more formally...
I had read that

Once connected, a TCP socket can only send and receive to/from the connected socket. So you'll need one TCP socket for each client in your application.

Since I'm trying to write a chat room for N users, my server must be able to communicate with different clients on TCP. As planned, I was trying to do thread "CheckConnectionsThr": but i really do not understand how to support multiple TCP connections. I would like to allocate stream to each client...

// the selector will tell you which socket has received data, and you can call "receive" on it directly

But... How do I know what socket finished receiving information?

If it's still not clear then play with a server that accepts more than 1 client. You'll understand the problem quickly

...I'm trying to create a server for more than 1 clients... And I got confused with the TCP.. With 1 client no problems.

[Saitei]

>>>Once connected, a TCP socket can only send and receive to/from the connected socket. So you'll need one TCP >>>socket for each client in your application.

This means that one and the same socket works for all clients? Oh... Still, I think in Russian and there are problems with the translation...

[Laurent]

Since I'm trying to write a chat room for N users, my server must be able to communicate with different clients on TCP. As planned, I was trying to do thread "CheckConnectionsThr": but i really do not understand how to support multiple TCP connections. I would like to allocate stream to each client...

The documentation of sf::SocketSelector shows how to accept multiple connections and then manage one socket per client in a selector.

But... How do I know what socket finished receiving information?

Have you read the tutorial and documentation? It's clearly explained, with examples.

I don't know what else to say. We can't teach you how networking works, only how to do it with SFML, and the documentation / tutorials already cover that quite well. You may need to read more articles about generic networking, that explain the concepts of sockets, selectors, etc. before you try to implement something with SFML.

[Saitei]

Since I'm trying to write a chat room for N users, my server must be able to communicate with different clients on TCP. As planned, I was trying to do thread "CheckConnectionsThr": but i really do not understand how to support multiple TCP connections. I would like to allocate stream to each client...

The documentation of sf::SocketSelector shows how to accept multiple connections and then manage one socket per client in a selector.

But... How do I know what socket finished receiving information?

Have you read the tutorial and documentation? It's clearly explained, with examples.

I don't know what else to say. We can't teach you how networking works, only how to do it with SFML, and the documentation / tutorials already cover that quite well. You may need to read more articles about generic networking, that explain the concepts of sockets, selectors, etc. before you try to implement something with SFML.

But nothing about threads for server

[Jesper Juhl]

But nothing about threads for server

Using threads for this will just complicate your code (big time) for no real gain.
Stick to a single thread unless there's a real performance problem you need to solve.

[Saitei]

It is a bad server?

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

typedef sf::Uint8 PacketType;
const PacketType MSG = 0;


const unsigned short PORT = 5000;
typedef std::unordered_map<sf::TcpSocket *, std::string> Clients;
Clients clients;
sf::TcpListener listner;
mutex sMutex;
bool ServerWorks = true;

void CheckConnectionsThr()
{
        sMutex.lock();
        cout<<"Thread \"CheckConnectionsThr\" started"<<endl;
        sMutex.unlock();

        sf::TcpSocket* nextClient = nullptr;
        while(ServerWorks)
        {
                if(nextClient == nullptr)
                {
                        sMutex.lock();
                        nextClient = new sf::TcpSocket;
                        sMutex.unlock();

                        nextClient->setBlocking(false);
                }
                if(listner.accept(*nextClient) == sf::Socket::Done)
                {
                        clients.insert(std::make_pair(nextClient, ""));
                        nextClient = nullptr;
                }
        }
}

void BroadCast(PacketType type, std::string& msg)
{
        for(Clients::iterator it=clients.begin(); it!=clients.end(); ++it)
        {
                sf::Packet pack;
                pack<<type<<msg;
                it->first->send(pack);
        }
}

void HandlePacketsThr()
{
       
        sMutex.lock();
        cout<<"Thread \"HandlePacketsThr\" started"<<endl;
        sMutex.unlock();
        while(ServerWorks)
        {
                for(Clients::iterator it=clients.begin(); it!=clients.end();)
                {
                        sf::Packet packet;
                        sf::Socket::Status status=it->first->receive(packet);
                        switch(status)
                        {
                                case sf::Socket::Done:
                                {
                                        PacketType type;
                                        packet >> type;
                                        if(type == MSG)
                                        {
                                                std::string msg;
                                                packet>>msg;
                                                sMutex.lock();
                                                cout<<msg<<endl;
                                                BroadCast(MSG,msg);
                                                sMutex.unlock();
                                        }
                                        ++it;
                                        break;
                                }
                                case sf::Socket::Disconnected:
                                {
                                        sMutex.lock();
                                        cout<<it->second<<" has been disconnected"<<endl;
                                        BroadCast(MSG, it->second+" has been disconnected\n");
                                        sMutex.unlock();
                                        it=clients.erase(it);
                                        break;
                                }
                                default:
                                {
                                        ++it;
                                }
                        }
                }
        }
}

int main()
{
        listner.listen(PORT);
        listner.setBlocking(false);
        cout<<"Starting the server..."<<endl;
        cout<<"Port:"<<PORT<<endl;
        thread checkConnectionsThr(CheckConnectionsThr);
        thread packetHandlerThr(HandlePacketsThr);
        checkConnectionsThr.detach();
        packetHandlerThr.detach();
        while(ServerWorks)
        {
                std::string s;
                cin>>s;
                if(s=="exit")
                {
                        ServerWorks = false;
                }
                else if(!s.empty())
                {
                        sMutex.lock();
                        BroadCast(MSG,s);
                        cout<<s<<endl;
                        sMutex.unlock();
                }
        }
        return EXIT_SUCCESS;
}

P.S: it's my code (but some peace i founded at the github). Now I'm going to create client...

[zsbzsb]

• Forget multithreading, you failed at it miserably
• You entirely used mutexts wrong
• None of your concurrently accessed varibles are protected
• Learn how memory management works in C++, you failed again with a memory leak
• 'using' statements at header level aren't good ideas
• Probably other things are wrong, you should really just stick to a single threaded server as already mentioned

[Saitei]

• Forget multithreading, you failed at it miserably
• You entirely used mutexts wrong
• None of your concurrently accessed varibles are protected
• Learn how memory management works in C++, you failed again with a memory leak
• 'using' statements at header level aren't good ideas
• Probably other things are wrong, you should really just stick to a single threaded server as already mentioned

You can paint detail my mistakes? I would be very grateful

[Ixrec]

You can paint detail my mistakes? I would be very grateful

There's no point going into great detail since most of the problems are as simple as "You don't know how X works" (it's harsh, but it's true).  But hopefully this helps a bit:

>You entirely used mutexts wrong
>None of your concurrently accessed varibles are protected

You seem to think that cout is the only thing that needs mutex protection.  I'm pretty sure cout does not need protection, and everything in your program that definitely needs protection (eg, clients) is unprotected.

Also, using the simple lock/unlock calls is often not the best or the safest approach.  As one simple example, using std::lock_guard will give you an RAII class that automatically unlocks the mutex on destruction, which is vital if you want exception safety.  And I'm not even beginning to scratch the surface of C++ standard locking mechanisms.

>Learn how memory management works in C++, you failed again with a memory leak

The example of this that really jumped out to me is:

nextClient = new sf::TcpSocket;
...
nextClient = nullptr;

This would be sensible code in a scripting language like Javascript, where setting a reference to null causes the previously referenced object to get garbage collected.  Nothing of the sort happens in C++.  Admittedly, it's clear that you want the global map to be the one "owning" the socket and this function to only point to things, but the fact that you're allocating the memory in that function without deallocating still causes a memory leak, even if you were hoping the std::map would somehow deal with it for you.

The naive fix here is to use the delete operator somewhere, since that's how you actually deallocate memory.  But really, you should almost never be dynamically allocating memory (ie, using the "new" operator) in modern C++ at all.  Using objects that follow the RAII idiom is far, far, far easier, safer and simpler in almost every situation.  Most sf:: and std:: objects already do.  Just declare an sf::TcpSocket normally and let its constructor/destructor worry about the low level stuff.  I believe using emplace() will allow you to construct the socket directly inside the map instead of having to construct one then insert a copy.

>'using' statements at header level aren't good ideas

Probably the least important issue here, but still true.  Basically, when you have a 'using' statement in a header, it also applies to every file that includes the header.  This means anyone (including yourself) who ever tries to use this header is more likely to have name conflict headaches.


Most importantly, as was already said, it's unlikely you'll gain anything from multiple threads other than a lot of bugs and headaches.  Multithreading is extremely hard, and doesn't benefit most simple programs.  Don't waste your time on it until you have a working program with a performance issue, and you've proven with a profiler that there is no other solution.  Knowing everything I said above and ten times more is absolutely mandatory before you can make threads do something useful.


But for the sake of ending this on a positive note:

it=clients.erase(it);

Kudos for using erase() correctly when iterating! Many newbies screw this up.

[Jesper Juhl]

It is a bad server?

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

typedef sf::Uint8 PacketType;
const PacketType MSG = 0;


const unsigned short PORT = 5000;
typedef std::unordered_map<sf::TcpSocket *, std::string> Clients;
Clients clients;
sf::TcpListener listner;
mutex sMutex;
bool ServerWorks = true;
 

Others have already commented on threading/locking and manual memory management issues.
I just want to point out one remaining thing that sticks out: the use of global variables.

You really want to avoid global variables. Search the forums if you want all the details (it has been discussed a lot). Here I just want to point to a few of the biggest problems.
Global variables

• reduce encapsulation (helps turn your code into spaghetti).
• gives you undefined construction/destruction order (between globals in different translation units).
• leads to bugs when objects have interdependencies (true for some SFML classes).

There is almost never any real/good reasons to use globals, so just don't do that.

[Saitei]

Oh! Thank you very much! Thanks to you, I realized how lousy server I wrote. Incidentally, I came up with a multithreaded server architecture ... I'll rewrite it.





Is it possible to send and receive packets at the same time? And what is meant by "blocking socket"?

[Ixrec]

Is it possible to send and receive packets at the same time?

Depends on what you mean by "at the same time".  If you mean "can I send packets without potentially missing ones being sent to me?", then yes.  I believe the OS's sockets will keep track of received data that you haven't processed yet so this shouldn't even be an issue, unless you use blocking sockets where you shouldn't.  If you mean "can I send and receive packets in the same program/thread/game loop?" then yes, see previous answer.  If you mean literally at the same moment in time, then kind of, in the sense that you can have one thread sending while another thread is paused midway through a receive.  The atomic parts of send and receive operations will probably always be sequential though.

And what is meant by "blocking socket"?

I'm pretty sure the tutorials explain this, but in programming when we say an operation is "blocking", that means it will prevent your program from continuing until that operation can be completed successfully.  The non-blocking equivalent will typically do nothing (and somehow tell you that it did nothing) if it can't be completed right away.

Sometimes you want blocking, sometimes you don't.  For instance, if you have a thread whose sole purpose is to listen for incoming connections and deal with their initial setup, you probably want it to listen on a blocking socket, since there's nothing for it to do until that socket has data to read.  But if you want to render a game or check user input in that same thread, you probably want a non-blocking socket so you can keep doing those things even when there's nothing new from the network.


By the way, if you have difficulty understanding the official tutorials and/or our posts, you may have to make an effort to find someone who can explain these things to you in your native language, or work on your English until you can understand them yourself.  It's important that you know this stuff before trying to write something as challenging as a multithreaded game server.

[Saitei]

Thanks!

oh... It is possible use TCP and UDP together?