2

I am trying to wrap the boost TCP using a new class in c++. Things work like a charm while I call the boost function directly. However I fail to call socket close while the close is wrap in a class function. Please help have a look on the following codes.

class defination:

typedef boost::shared_ptr<tcp::socket> Socket;
class TCPConnector{
public :
    bool isConnected;
    Socket sock;
    string ip;
    int port;
    TCPConnector(string ip, int port);

    void Close();
    bool Connect();
};

functions:

TCPConnector::TCPConnector(string ip,int port):ip(ip),port(port)
{

}

void TCPConnector::Close() {
    boost::system::error_code error;
    if (!isConnected)
        return;
    isConnected = false;

    try {

        sock->shutdown(boost::asio::ip::tcp::socket::shutdown_both, error);
        cout << "ShutDown" << endl;
        if (error)
            throw boost::system::system_error(error);
        sock->close(error);
        if (error)
            throw boost::system::system_error(error);
    } catch (exception& e) {
        cout << "#TCPConnector::Close()#" << e.what() << endl;
    }
}

Main Function:

int main(int argc, char* argv[]) {
    try {
        TCPConnector* conn = new TCPConnector("127.0.0.1",8088);

        for (int i = 0; i < 2; i++) {
            boost::asio::io_service io_service;
            tcp::resolver resolver(io_service);
            tcp::resolver::query query(tcp::v4(), "127.0.0.1", "8088");
            tcp::resolver::iterator endpoint_iterator = resolver.resolve(query);
            conn->sock.reset(new tcp::socket(io_service));
            conn->sock->connect(*endpoint_iterator);
            cout << "Connected" << endl;
            boost::thread acceptorThread(boost::bind(receive,conn));
            sleep(1);
            unsigned char msg[8] = { 0, 6, 55, 56, 55, 56, 55, 0 };
            boost::system::error_code error;
            try {

                boost::asio::write(*conn->sock, boost::asio::buffer(msg, 8),
                        boost::asio::transfer_all(), error);
                cout << "Sent" << endl;
                if (error)
                    throw boost::system::system_error(error);
                conn->sock->shutdown(boost::asio::ip::tcp::socket::shutdown_both,
                        error);
                if (error)
                    throw boost::system::system_error(error);




                conn->sock->close(error);//close socket directly , the result is ok
                //conn->Close();// close by calling functions, it causes problems.





                cout << "Closed" << endl;
                if (error)
                    throw boost::system::system_error(error);
                io_service.stop();
            } catch (std::exception& e) {
                std::cerr << "Exception in thread: " << e.what() << "\n";
            }
            cout << "Sleep" << endl;
            sleep(2);
            cout << "Wake up" << endl;
        }
    } catch (std::exception& e) {
        std::cerr << e.what() << std::endl;
    }
    return 0;
}

These 2 lines give the different behaviours. I don't know why the second one will cause problem.

conn->sock->close(error);//close socket directly , the result is ok
conn->Close();// close by calling functions, it causes problems.

mutex: Invalid argument was printed on

sock->close(error);
        if (error)
            throw boost::system::system_error(error);

Is the problem related to shared_ptr? or I missed something important to close the socket?

Thanks for any suggestion.

Sam Miller
  • 23,808
  • 4
  • 67
  • 87
HenryLok
  • 81
  • 1
  • 7
  • The `io_service` should outlive the `socket`. Try moving the `io_service` outside the for loop, or calling `conn->sock.reset();` at the end of the for loop. – Fraser Feb 28 '13 at 03:11
  • Thanks for your reply. In the first loop, the error is thrown after `conn->Close();` , so the program is not able to continue. or you mean call `conn->sock.reset()` before the `conn->Close();` ? – HenryLok Feb 28 '13 at 03:21
  • @Fraser Thanks a lot! After adding `conn->sock.reset();` before the `conn->Close();`, the code works now. May I try to explain and you help assess whether it is right. As the `conn->sock.reset(new tcp::socket(io_service));` runs after the `io_service.stop();`, so the `io_service` live longer than the socket(which is closed in the before loop). but why this will cause mutex error and why the `io_service` has to live longer than the socket? Really thanks a lot! – HenryLok Feb 28 '13 at 03:29
  • @Fraser One more question, the `reset` function of the shared_ptr will run the destructor of the socket? if yes, why the sock can be used after the reset is called? Thanks – HenryLok Feb 28 '13 at 03:59
  • 1
    Not always, `reset` is equivalent to `swap`. It runs the destructor of the managed object only if the `use_count` is 1 when called. In this case (since the `use_count` *is* 1) it does run the destructor of the socket. If you call `reset()` (equivalent to setting the ptr to `nullptr`) at the end of the loop, you run the destructor of the socket while the `io_service` is still valid. If you don't do that, the `reset` near the start of the loop executes the destructor of the previous iteration's socket before assigning the new value - by which time the previous iter's `io_service` is invalid. – Fraser Feb 28 '13 at 04:18
  • Thanks a lot! would you consider answering the question and let me accept it? – HenryLok Feb 28 '13 at 07:40

1 Answers1

3

The problem is that the io_service should outlive the socket.

On all but the first iteration of the for loop, the statement conn->sock.reset(new tcp::socket(io_service)); calls the destructor of the previous iteration's socket. This destructor accesses elements of the previous iteration's io_service (specifically its mutex) which by that point have themselves been destroyed.

To fix this, you can move the io_service outside the for loop, or you can call conn->sock.reset(); at the end of the for loop in order to invoke the socket's destructor while the io_service is still valid.

Fraser
  • 74,704
  • 20
  • 238
  • 215