6

Alright, this is my current code snippet:

namespace bai = boost::asio::ip;
bai::tcp::socket tcp_connect(std::string hostname, std::string port) {
    try {
        boost::asio::io_service io_service;
        bai::tcp::resolver resolver(io_service);

        // we now try to get a list of endpoints to the server 
        bai::tcp::resolver::query query(hostname, port);
        bai::tcp::resolver::iterator endpoint_iterator = resolver.resolve(query);
        bai::tcp::resolver::iterator end;

        // looking for a successful endpoint connection
        bai::tcp::socket socket(io_service);
        boost::system::error_code error = boost::asio::error::host_not_found;
        while (error && endpoint_iterator != end) {
            socket.close();
            socket.connect(*endpoint_iterator++ , error);
        }

        if (error) throw boost::system::system_error(error);

        return socket;
    } catch (std::exception &ex) {
        std::cout << "Exception: " << ex.what() << "\n";
    }
}

Which should return a boost::asio::ip::tcp::socket connected to hostname on port. However I get presented with a shitload of incomprehensible boost::noncopyable errors. But my question is, how should I pass around these sockets then? What's wrong with this?

NmdMystery
  • 2,778
  • 3
  • 32
  • 60
orlp
  • 112,504
  • 36
  • 218
  • 315

2 Answers2

15

socket can't be copied. Use a boost::shared_ptr<bai::tcp::socket> instead. If you could copy a socket you'd have all sorts of funny issues if you ended up with two socket instances representing the same underlying OS socket - so it makes sense that copying (and therefore return by value, pass by value) is not allowed.

Erik
  • 88,732
  • 13
  • 198
  • 189
  • I have never used `share_ptr` before, like this? `boost::shared_ptr tcp_connect(std::string hostname, std::string port)` – orlp Mar 24 '11 at 21:27
  • `boost::shared_ptr tcp_connect(....)` and then `boost::shared_ptr socket(new bai::tcp::socket(io_service));` – Erik Mar 24 '11 at 21:28
  • Alright, that works. This does mean that I have to change every instance of `socket.some_method()` to `(*socket).some_method()` right? – orlp Mar 24 '11 at 21:37
  • Errr, `socket->some_method()` works fine, but *socket is needed for `boost::asio::read()`. – orlp Mar 24 '11 at 21:41
  • 1
    Out of interest, does anyone know why the documentation suggests socket has a constructor that would allow this kind of operation? (see the last override described at http://www.boost.org/doc/libs/1_47_0/doc/html/boost_asio/reference/basic_stream_socket/basic_stream_socket.html) As a novice C++ programmer, I'm not sure I follow the distinction between move-constructing and copy-constructing -- I'd assumed the distinction was that the original would be left unusable in a move construct operation? – Jules Nov 24 '12 at 08:24
  • @Jules You are tright but it looks that this is still an issue even if the ticket is closed 16 months ago as of today. http://stackoverflow.com/a/19193337/888576 – Jean Davy Jan 10 '15 at 20:05
  • Not being the c++ novice I was when I wrote that comment, I now see that the code quoted would try to create a copy of the socket to return and then destroy the original. Returning std::move(socket) rather than socket should help; the function would then be able to use the move constructor of the object declared in the caller to receive the returned value, rather than trying to copy it. In fact, this should probably be the answer to this question... – Jules Jan 11 '15 at 10:05
4

The code:

return socket;

attempts to make a copy of socket to return, and then destroy the original socket when the function exits. Unfortunately, sockets cannot be copied (they manage an underlying operating system resource that must be closed, so the system must ensure only one reference to that resource exists, otherwise things would go badly wrong when the first copy went out of scope).

As suggested in the other answer, you could use a pointer to return an object created on the heap (which should be managed either with a shared_ptr, or more efficiently if you are only using it in a single place a unique_ptr), or if you are using C++11 you could use the move constructor for the return value:

return std::move (socket);

This would avoid the necessity to use heap allocation and pointers, so is probably the preferable solution.

Jules
  • 14,841
  • 9
  • 83
  • 130