How can I lock one thread from another?

[TheRabbit]

I'm trying to combat a race condition that occurs when a user connects to my server. Because the connection function is spawned in a thread, the main thread continues to see an "incoming connection" until one of the threads latches, which often results in 4 or 5 extra threads being created. I thought three fix for this would be fairly simple, just lock a mutex right before the thread launches, and then unlock the mutex add soon as a connection latches. But, mutexs throw an exception when you try and unlock in a different thread than you locked in....

So instead I did something like this: (please ignore syntax, I'm typing this up on my phone from memory)

 bool connLock = false;
...
if (selector.isReady(listener) && !connLock)
{
    std::thread(login,this);
    connLock = true;
}

...
login()
{

    if (listener.accept(client->socket) != sf::Socket::Done)
    {
      connLock = false;
      Return;
    }
    connLock = false;
....
 

I probably messed up some of those lines of code, sorry.
Anyway, this works pretty good, it lets my sever thread keep chugging away on other things while the login thread does it thing, but there has to be a better way to do this, right?

Any suggestions?

[Laurent]

It's really weird to check if the listener has an incoming connection and then accept it in another thread. To me, the obvious solution is the following:

if (selector.isReady(listener))
{
    if (listener.accept(client->socket) == sf::Socket::Done)
        std::thread(login, this).detach();
}

[binary1248]

The functionality you are looking for is known as a semaphore or to some extent also condition variable. SFML only provides mutex support. You need to understand the idea behind a mutex, they stand for mutual exclusion and are a way to prevent multiple threads from accessing shared resources at the same time. As such, the more intuitive interface would probably rename lock() and unlock() to acquire() and release(). The same thread that acquires a resource to use must be the one who releases it, else a mutex won't make much sense. SFML mutexes are recursive as well so this is even more important in that sense. If you want to perform inter-thread synchronization then you should consider the other possibilities I mentioned. Since you are using std::thread, I assume you also have support for std::condition_variable, so you might try that.

But as Laurent said, before overengineering something, you should consider much simpler solutions first .

[TheRabbit]

It's really weird to check if the listener has an incoming connection and then accept it in another thread. To me, the obvious solution is the following:

if (selector.isReady(listener))
{
    if (listener.accept(client->socket) == sf::Socket::Done)
        std::thread(login, this).detach();
}

Well, the reason I wasn't doing it this way is because of that client pointer. I store a client list, and use iterators to access them, and I don't want to add a client to the list until they've been authenticated (so that I'm not sending chat messages intermingled with the hand shake). What I could do is generate the blank client right there and do the socket connection, but keep the push inside my login function by just passing it in. It adds an extra copy function to the whole process, but that isn't significant in the scheme of things.
Thanks, I'll give this a shot.

[Laurent]

Then the correct way of handling it would be (pseudo-code):

if (listener ready)
{
    TcpSocket socket;
    if (listener.accept(socket))
        thread(login, socket);
}

void login(TcpSocket& socket)
{
    if (login successful)
        clients.add(new Client(socket));
}

[TheRabbit]

Hey that's an even better idea, just pass in the accepted socket instead of an entire blank client that only has a socket. I'll have to add a client function that moves a socket into the client.socket though.

Edit: Now that I'm at my computer, here's how it does it:

class Client
{
public:
    //Default constructor-- should probably never be used...
        Client()
    : socket(new sf::TcpSocket)
    {
                ID = -1;
    }

        //Move constructor
    Client(Client&& source)
    : socket(std::move(source.socket))
    {
                ID = source.ID;
                name = source.name; //client's user name
    }

    //Move assignment operator
    Client& operator= (Client&& source)
    {
        ID = -1;
        socket = std::move(source.socket);
        return *this;
    }

        int loadClient(const std::string &clientName); //looks up a user based on their name

public:
        std::unique_ptr<sf::TcpSocket> socket; //socket handling communication to this client
        std::string name; //client's user name
        int ID; //client's unique ID
};

class Server
{
public:
        Server(): mWindow(sf::VideoMode(400, 400), "Server Connection", sf::Style::Close){};//force window to certain size
        void run();
        sf::RenderWindow mWindow;

public:

private:
        void processEvents(); //handles input
        void loginAttempt(std::unique_ptr<sf::TcpSocket> &inputSocket); //handles login attempts

private:
        std::list<Client> clientList; //sockets we will use to communicate
        sf::SocketSelector selector; //used to handle multiple clients
        sf::TcpListener listener; //will listen on a specific port for an incoming connection

};

void Server::run()
{
        if (listener.listen(port) != sf::Socket::Done) {
                std::cout << "Failed to listen to port " + std::to_string(port) + ". Something else already attached to port?\n";
                return;}
        std::cout << "Currently listening to port " + std::to_string(port) + ". Waiting for first connection...\n";

        selector.add(listener); //put the new connection listener into our selector
        listener.setBlocking(false); //we no longer want to block when checking for new connections

        while(mWindow.isOpen()) //main server hosting loop
        {      
                if (selector.wait(sf::seconds(1.f))) //server sits and waits for an event to occur on the selector
                {
                        if(selector.isReady(listener)) //we have a new incoming connection
                        {
                               
                                std::unique_ptr<sf::TcpSocket> tempSocket(new sf::TcpSocket);
                                if (listener.accept(*tempSocket) == sf::Socket::Done)
                                        std::thread (&Server::loginAttempt, this, std::move(tempSocket)).detach();//create a new thread to handle the login events
                        }
...

void Server::loginAttempt(std::unique_ptr<sf::TcpSocket> &inputSocket)
{
        Client *client = new Client();
        client->socket = std::move(inputSocket);
        bool success = false; //indicates login success or failure
...
        if (!success) //client failed to pass all conditions to connect to the server, so we remove them
        {
                selector.remove(*(client->socket)); //stop listening to the client that disconnected
                std::cout << "Client failed to login.\n";
                return;
        }
        else
        {
                //client passed all conditions to connect to server
                clientList.push_back(std::move(*client)); //push the client we made into the list!
                return;
        }
}