How should I delete a child object from within a parent's slot? Possibly boost::asio specific.
- by kaliatech
I have written a network server class that maintains a std::set of network clients. The network clients emit a signal to the network server on disconnect (via boost::bind). When a network client disconnects, the client instance needs to be removed from the Set and eventually deleted. I would think this is a common pattern, but I am having problems that might, or might not, be specific to ASIO.
I've tried to trim down to just the relevant code:
/** NetworkServer.hpp **/
class NetworkServices : private boost::noncopyable
{
public:
NetworkServices(void);
~NetworkServices(void);
private:
void run();
void onNetworkClientEvent(NetworkClientEvent&);
private:
std::set<boost::shared_ptr<const NetworkClient>> clients;
};
/** NetworkClient.cpp **/
void NetworkServices::run()
{
running = true;
boost::asio::io_service::work work(io_service); //keeps service running even if no operations
// This creates just one thread for the boost::asio async network services
boost::thread iot(boost::bind(&NetworkServices::run_io_service, this));
while (running)
{
boost::system::error_code err;
try
{
tcp::socket* socket = new tcp::socket(io_service);
acceptor->accept(*socket, err);
if (!err)
{
NetworkClient* networkClient = new NetworkClient(io_service, boost::shared_ptr<tcp::socket>(socket));
networkClient->networkClientEventSignal.connect(boost::bind(&NetworkServices::onNetworkClientEvent, this, _1));
clients.insert(boost::shared_ptr<NetworkClient>(networkClient));
networkClient->init(); //kicks off 1st asynch_read call
}
}
// etc...
}
}
void NetworkServices::onNetworkClientEvent(NetworkClientEvent& evt)
{
switch(evt.getType())
{
case NetworkClientEvent::CLIENT_ERROR :
{
boost::shared_ptr<const NetworkClient> clientPtr = evt.getClient().getSharedPtr();
// ------ THIS IS THE MAGIC LINE -----
// If I keep this, the io_service hangs. If I comment it out,
// everything works fine (but I never delete the disconnected NetworkClient).
// If actually deleted the client here I might expect problems because it is the caller
// of this method via boost::signal and bind. However, The clientPtr is a shared ptr, and a
// reference is being kept in the client itself while signaling, so
// I would the object is not going to be deleted from the heap here. That seems to be the case.
// Never-the-less, this line makes all the difference, most likely because it controls whether or not the NetworkClient ever gets deleted.
clients.erase(clientPtr);
//I should probably put this socket clean-up in NetworkClient destructor. Regardless by doing this,
// I would expect the ASIO socket stuff to be adequately cleaned-up after this.
tcp::socket& socket = clientPtr->getSocket();
try {
socket.shutdown(boost::asio::socket_base::shutdown_both);
socket.close();
}
catch(...) {
CommServerContext::error("Error while shutting down and closing socket.");
}
break;
}
default :
{
break;
}
}
}
/** NetworkClient.hpp **/
class NetworkClient : public boost::enable_shared_from_this<NetworkClient>, Client
{
NetworkClient(boost::asio::io_service& io_service,
boost::shared_ptr<tcp::socket> socket);
virtual ~NetworkClient(void);
inline boost::shared_ptr<const NetworkClient> getSharedPtr() const
{
return shared_from_this();
};
boost::signal <void (NetworkClientEvent&)> networkClientEventSignal;
void onAsyncReadHeader(const boost::system::error_code& error,
size_t bytes_transferred);
};
/** NetworkClient.cpp - onAsyncReadHeader method called from io_service.run()
thread as result of an async_read operation. Error condition usually
result of an unexpected client disconnect.**/
void NetworkClient::onAsyncReadHeader( const boost::system::error_code& error,
size_t bytes_transferred)
{
if (error)
{
//Make sure this instance doesn't get deleted from parent/slot deferencing
//Alternatively, somehow schedule for future delete?
boost::shared_ptr<const NetworkClient> clientPtr = getSharedPtr();
//Signal to service that this client is disconnecting
NetworkClientEvent evt(*this, NetworkClientEvent::CLIENT_ERROR);
networkClientEventSignal(evt);
networkClientEventSignal.disconnect_all_slots();
return;
}
I believe it's not safe to delete the client from within the slot handler because the function return would be ... undefined? (Interestingly, it doesn't seem to blow up on me though.) So I've used boost:shared_ptr along with shared_from_this to make sure the client doesn't get deleted until all slots have been signaled. It doesn't seem to really matter though.
I believe this question is not specific to ASIO, but the problem manifests in a peculiar way when using ASIO. I have one thread executing io_service.run(). All ASIO read/write operations are performed asynchronously. Everything works fine with multiple clients connecting/disconnecting UNLESS I delete my client object from the Set per the code above. If I delete my client object, the io_service seemingly deadlocks internally and no further asynchronous operations are performed unless I start another thread. I have try/catches around the io_service.run() call and have not been able to detect any errors.
Questions:
Are there best practices for deleting child objects, that are also signal emitters, from within parent slots?
Any ideas as to why the io_service is hanging when I delete my network client object?
