I am making a HTTP server and I want to pass a socket client class to thread to handle the request. The problem is that the destructor gets called right after starting the thread so the socket gets closed. The simplified version of code looks like this.
while (true) {
TcpSocket client = m_socket.accept();
std::thread clientThread([this, &client] { this->handleClient(client); });
clientThread.detach();
}
I tried making a function that accepts a connection and passes it to a thread with a given function but I can't get it to work. I am using a tuple here because you can't pass a parameter pack to lambda in c++17.
template<typename Function, typename ...Args>
std::thread TcpListener::acceptAndPassToThread(Function&& function, Args&&... args)
{
int socketDescriptor = accept(m_socketDescriptor, nullptr, nullptr);
std::tuple arguments = std::make_tuple(std::forward<Args>(args)...);
std::thread clientThread([socketDescriptor, function, arguments] {
TcpSocket client = TcpSocket(socketDescriptor);
auto callArguments = std::tuple_cat(std::make_tuple(client), arguments);
std::apply(function, callArguments);
});
return clientThread;
}
I could do something like this but I would prefer not to use socket descriptors outside the class.
while (true)
{
int clientDescriptor = m_socket.acceptDescriptor();
std::thread clientThread([this, clientDescriptor] {
TcpSocket client = TcpSocket::fromDescriptor(clientDescriptor);
this->handleClient(client);
});
clientThread.detach();
}
Also what would be the proper way to call the TcpListener::acceptAndPassToThread function with a class member function.
Your RAII
class TcpSocket
should really be move only because it is logically wrong to make copies. Of course you could implement it like a std::shared_ptr
but then the value semantics are a bit misleading. It would then probably be better to call it something that reflects its shared nature.
To make a class move only you delete the copy constructor and the copy assignment operator and provide move versions of both. Something like this:
class TcpSocket
{
public:
// ...
~TcpSocket() { this->close(); }
TcpSocket(int descriptor = -1): descriptor(descriptor) {}
// ban copying
TcpSocket(TcpSocket const&) = delete;
TcpSocket& operator=(TcpSocket const&) = delete;
// implement move so the moved from object will
// not close this descriptor
TcpSocket(TcpSocket&& other): TcpSocket()
{
std::swap(descriptor, other.descriptor);
}
// same with move assignment
// this closes any current descriptor but you may
// want to throw an exception if this descriptor
// is currently in use.
TcpSocket& operator=(TcpSocket&& other)
{
if(&other != this)
this->close(); // or throw?
std::swap(descriptor, other.descriptor);
return *this;
}
private:
void close()
{
if(descriptor != -1)
::close(descriptor);
descriptor = -1;
}
int descriptor = -1;
};
Then move the socket into the thread:
while (true) {
TcpSocket client = m_socket.accept();
std::thread clientThread([this, client = std::move(client)] mutable {
this->handleClient(std::move(client));
});
clientThread.detach(); // looks dangerout?
}