sf::SocketTCP.Send() & sf::SocketTCP.Receive() Threadsaf

[affiliated]

Hello,

I started to wirte a multithread TCP Server and have a question to the following functions:

sf::SocketTCP.Send()
sf::SocketTCP.Receive()

Are this functions Threadsafe? It does not make any sense if i hade a send and a receive thread for each client if this functions are not threadsafe. If they are not i have to use a mutex to "make them safe" but then the send or the recive thread locks the object and the other one can't act as it should?

Tanks,

Affiliated

[Laurent]

From SFML point of view, I think these functions can be considered thread-safe.
From the OS point of view I don't really know, but I think they are too.

[Ceylo]

From SFML point of view, I think these functions can be considered thread-safe.
From the OS point of view I don't really know, but I think they are too.

I'm currently working with threads and network and.. I was asking the same questions to myself. What does "I think" and "I don't really know" mean ?

Do I have to care about protecting the sf::Socket objects among the different threads or not ? Having to check if the socket is protected each time it's used, and make sure there won't be two unlimited inter-blocking locks is somewhat heavy to handle (it makes the code look like a big soup...).

[Laurent]

What does "I think" and "I don't really know" mean ?

It means respectively "not tested" and "no idea"

Do I have to care about protecting the sf::Socket objects among the different threads or not ?

I can't tell you, you'll have to test yourself.

[Ceylo]

Could you give some hints about how I am supposed to make these tests as concurrent access can happen randomly ? (besides how can I make sure the behavior will be the same on every supported OS ?)

Otherwise I think I'm gonna add locks into the TCPSocket class.

[Laurent]

I think the best solution for you is to assume that SFML is not multithread-safe, and make your own code multithread-safe. It will be much easier.

[Ceylo]

Aren't there any plan to make the network package thread-safe ?

If I want to protect my objects manually, I've to write something like this :

Code: [Select]


xxx.Lock();
sharedObject.DoSomething();
xxx.Unlock();
or

Code: [Select]


{
    sf::Lock l(xxx);
    sharedObject.DoSomething();
}

And the things get even worth when a shared object is used in conditions : I've to get the result of the function between lock/unlock before using the condition, then do the condition test. Same when a shared object is used in a return statement.

Whereas if it was done on SFML's side.. of course it would be quite easier for me, but it wouldn't be too tricky for you. Besides it's not like protecting a function used hundreds of times per second. I don't think the protection overhead would be noticeable in this case.

I could obviously patch SFML myself but it would be a pain to maintain.


Edit: note that I can't protect the members for the whole function lifetime either because some are already blocking the thread by waiting on a selector, and in the meantime, another thread could request the use of one of the protected object to send a message to this selector => inter-blocking.

I wonder whether this isn't related to a bad conception though. What I've done is launching a TCP server in a thread which waits for messages from clients. When I wish to stop listening, the only way I found was to send a "kill" message to the server (I don't think killing the thread is fine for closing connections and all this kind of stuff). But.. in the meantime I'm writing this, I've been thinking of another solution : instead of waiting indefinitely for the selector to return, I could set a timeout of.. let's say one second, which would let me check if a boolean "shouldStopServer" has been set to true in the meantime. Thus I wouldn't have to use the socket, no more sharing... and no more inter-locking.

This issue with the ease of use of the network package remains though.

[Tank]

When I wish to stop listening, the only way I found was to send a "kill" message to the server (I don't think killing the thread is fine for closing connections and all this kind of stuff).

You can just close the listener socket. The selector will return and you can then check the socket if it's still in a valid state.

[Ceylo]

When I wish to stop listening, the only way I found was to send a "kill" message to the server (I don't think killing the thread is fine for closing connections and all this kind of stuff).

You can just close the listener socket. The selector will return and you can then check the socket if it's still in a valid state.

Hum.. not that bad, thanks !

[Ceylo]

Up ! No news from Laurent about the network package safety and usability ?

[Merakon]

I had the same question today.

Has anyone determined whether SFML's SocketTCP class is thread-safe, at least with regard to Send and Receive?

If not, has anyone made any code modifications that they could share?

P.S. Laurent: I like SFML a lot!

[Laurent]

Has anyone determined whether SFML's SocketTCP class is thread-safe, at least with regard to Send and Receive?

No, as far as I know.
Why do you need Send and Receive to be multithread-safe?

[Ceylo]

Has anyone determined whether SFML's SocketTCP class is thread-safe, at least with regard to Send and Receive?

No, as far as I know.
Why do you need Send and Receive to be multithread-safe?

To be able to send data on a thread, and receive on another thread ? But I believe this is a bad practice, but I also believe that.. depending on the context, this could be useful and the most efficient way.

[Laurent]

I don't think that a socket can perform multiple tasks in parallel. Things have to be done sequentially. That's why I wonder why people want to use the same socket in multiple threads, I can't find a situation where this can be useful.

[Merakon]

The reason that I want to Send and Receive on the same socket is simplicity of code. If I can't Send and Receive on the same socket, that means each client needs to create two sockets to connect to the server and the server needs two sockets per client.

Anyway, I seem to have come to a simple solution by using SetBlocking(false) on the socket. This prevents the Receive call from causing a deadlock, and allows me to use a Mutex to protect the sf::Socket object. I hope this doesn't cause other problems, but for now it works. Here is the code, for anyone's benefit and/or feedback:

Code: [Select]


// netclient.h

// Note: I also include a project .h which uses namespace std
#include <SFML/Network.hpp>
#include <SFML/System.hpp>

class NetClient
{
public:
NetClient();
virtual ~NetClient();

bool ConnectToServer();

bool SendMessage(string message);

sf::SocketTCP   m_Socket;
sf::Mutex       m_SocketMutex;

protected:
sf::Thread    * m_pThread;
};


Code: [Select]


// netclient.cpp

#include "netclient.h"
#include <iostream>

NetClient::NetClient() : m_pThread(NULL)
{
}

NetClient::~NetClient()
{
if (this->m_Socket.IsValid())
{
this->m_SocketMutex.Lock();
this->m_Socket.Close();
this->m_SocketMutex.Unlock();
}
if (this->m_pThread != NULL)
{
this->m_pThread->Wait();
delete this->m_pThread;
}
}

void Receiver(void* UserData)
{
NetClient * netClient = static_cast<NetClient*>(UserData);

bool keepGoing = true;
while (keepGoing)
{
sf::Socket::Status status;
sf::Packet packet;
netClient->m_SocketMutex.Lock();
status = netClient->m_Socket.Receive(packet);
netClient->m_SocketMutex.Unlock();

if (status == sf::Socket::Disconnected)
{
keepGoing = false;
}
else if (status == sf::Socket::Error)
{
cout << "Encountered an error reading from socket.\n" << endl;
keepGoing = false;
}
else if (status == sf::Socket::Done)
{
string message;
packet >> message;
cout << "Server says : \"" << message << "\"" << endl;
}
}
}

bool NetClient::ConnectToServer()
{
const int Port = 2839;

// Ask for server address
sf::IPAddress ServerAddress;
do
{
cout << "Type the address or name of the server to connect to : ";
cin  >> ServerAddress;
}
while (!ServerAddress.IsValid());
cin.ignore(10000, '\n');

// Connect to the server
if (this->m_Socket.Connect(Port, ServerAddress) != sf::Socket::Done)
return false;

this->m_Socket.SetBlocking(false);

this->m_pThread = new sf::Thread(&Receiver, this);
thus->m_pThread->Launch();

return true;
}

bool NetClient::SendMessage(string message)
{
if (this->m_Socket.IsValid() == false)
{
cout << "Error: Can't send network message. Socket is invalid." << endl;
return false;
}

sf::Packet packet;
packet << message;
this->m_SocketMutex.Lock();
this->m_Socket.Send(packet);
this->m_SocketMutex.Unlock();
return true;
}