3

When I call socket() inside a std::thread() it returns a socket descriptor of 1. Calls to std::cout than send the text meant for the terminal to the server. When I added cout << ""; prior to the call to socket(), descriptors for stdin/out/err for are created and socket() returns 3.

From http://www.cplusplus.com/reference/iostream/cout/:

In terms of static initialization order, cout is guaranteed to be properly constructed and initialized no later than the first time an object of type ios_base::Init is constructed, with the inclusion of <iostream> counting as at least one initialization of such objects with static duration.

By default, cout is synchronized with stdout (see ios_base::sync_with_stdio).

std::cout should already be initialized and synchronized to stdout which explains why std::cout is sending the message to the server and not the terminal.

I'm wondering if calling std::thread() closes the stdin/out/err descriptors in the new thread or if these descriptors don't exist in the thread since the thread wasn't created by the terminal or initd?

I'm running on RHEL 6.4 using GCC 4.8.2. I've included my client code below with the additional cout << ""; commented out for completeness.

Client.cpp:

#include "Client.hpp"

#include <cstdlib>
#include <cstring>
#include <iostream>
#include <thread>
#include <vector>
#include <algorithm>
#include <sstream>
#include <functional>

#include <sys/types.h>
#include <sys/socket.h>
#include <netdb.h>
#include <unistd.h>

using namespace std;

Client::Client( const string& hostname, const string& port ) {
    this->hostname = hostname;
    this->port = port;
}

Client::~Client() { close( fd ); }

void Client::operator()() {
    struct addrinfo hints;
    memset( &hints, 0, sizeof( struct addrinfo ) );

    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_flags = 0;
    hints.ai_protocol = 0;

    struct addrinfo *result;
    int ret = getaddrinfo( hostname.c_str(), port.c_str(), &hints, &result );
    if( ret != 0) {
        cerr << "getaddrinfo failed: " << gai_strerror( ret ) << endl;
        return;
    }

    // cout << ""; // prevents socket() from returning 1 and redefining cout

    struct addrinfo *rp = NULL;
    for( rp = result; rp != NULL; rp = rp->ai_next ) {
        fd = socket( rp->ai_family, rp->ai_socktype, rp->ai_protocol );
        if( fd == -1 ) {
            continue;   /* Error */
        }
    
        if( connect( fd, rp->ai_addr, rp->ai_addrlen ) != -1) {
            break;      /* Success */
        }

        close( fd );        /* Try again */
    }

    if( rp == NULL ) {
        cerr << "Failed to connect to " << hostname << ":" << port << endl;
        return;
    }

    freeaddrinfo( result );

    cout << "Starting echo client" << endl;

    int i = 0;
    do {
        stringstream ss;
        ss << "Thread " << this_thread::get_id() << ": Message #" << ++i;
        string str = ss.str();

        _send( str.c_str() );
        string msg = _recv();

        //cout << "Thread " << this_thread::get_id() << " received message: " << msg << endl;
    } while ( i < 10 );

    cout << "Stopping echo client" << endl;

    close( fd );
}

string Client::_recv( ) const {
    const int BUF_SIZE = 1024;
    char* buf = ( char * ) malloc( sizeof( char) * BUF_SIZE );
    memset( buf, '\0', sizeof( char ) * BUF_SIZE );

    int bytes = recv( fd, buf, BUF_SIZE, 0 );
    if( bytes < 0 ) {
        perror( "recv failed" );
    }

    string msg = string( buf );

    free( buf );

    return msg;
}

void Client::_send( const string buf ) const {
    if( send( fd, buf.c_str(), buf.length(), 0 ) < 0 ) {
        perror( "send failed" );
    }
}
void usage() {
    cerr << "Usage: client <hostname> <port>" << endl;
    cerr << "   hostname - server name listening for incoming connctions" << endl;
    cerr << "   port - internet port to listen for connections from" << endl;
}

int main( int argc, char* argv[] ) {
    if( argc < 3 ) {
        cerr << "Not enough arguments!" << endl;
        usage();
        return EXIT_FAILURE;
    }

    vector<thread> clients;
    for( int i = 0; i < 1; i++ ) {
        clients.push_back( thread( ( Client( argv[1], argv[2] ) ) ) );
    }

    for_each( clients.begin(), clients.end(), mem_fn( &thread::join ) );

    return EXIT_SUCCESS;
}

Client.hpp:

#ifndef __CLIENT_HPP__
#define __CLIENT_HPP__

#include <string>

class Client {
    private:
        int fd;
        std::string hostname;
        std::string port;
        std::string _recv( ) const;
        void _send( const std::string buf ) const;

    public:
        Client( const std::string& hostname, const std::string& port);
        ~Client();
        void operator()();
};

#endif
Community
  • 1
  • 1
walsht
  • 196
  • 1
  • 10
  • 1
    Please show the .hpp file as well – pilcrow Feb 13 '15 at 18:27
  • 1
    This is a standard [Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) violation. Your class holds a resource (`fd`) that is destroyed in the destructor, you should either implement or forbid copy/move construction/assignment. Also, the call to `freeaddrinfo` is after the `return` in the error conditional, resulting in a memory leak when connection is unsuccessful. – Casey Feb 13 '15 at 19:52

1 Answers1

5

This normally happens if you accidentally close your stdout, i.e. there is a spurious close(1);. Then the FD will legitimately be number 1. This may well be elsewhere in the program.

I've had this several times, and normally find it with gdb and setting a breakpoint on close().

Your destructor looks suspicious:

Client::~Client() { close( fd ); }

Shouldn't you be setting fd to -1 in the constructor, and carefully setting fd to -1 wherever else you close it, and not doing this close if fd==-1? Currently creating a Client and destroying it will close a random fd.

abligh
  • 24,573
  • 4
  • 47
  • 84
  • All the code is posted in the question. I'm compiling with `g++ -std=gnu++11 -pthread -ggdb -Wall -c Client.cpp; g++ -std=gnu++11 -pthread -ggdb -Wall -o client Client.o;` and launching the program using `client localhost 9850`;`. I'm not using gdb to launch the program but I do compile it with debugging info enabled. – walsht Feb 13 '15 at 18:51
  • Initializing fd to -1 seems to have done the trick. I also added an `if( fd != -1 ) { close( fd ); }` to the destructor to prevent closing a random descriptor. I'll add `fd = -1;` everywhere I call `close()` to eliminate closing a random descriptor when the object is destructed. – walsht Feb 13 '15 at 19:04
  • For what it's worth, the spurious close is not "elsewhere in the code." It's at the end of the `for` loop in `main` when the Client object goes out of scope after having been copied to a new object for use by the new thread. – pilcrow Feb 16 '15 at 14:59