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 ;)
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...
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.
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.