Merge pull request #3904 from IvanSavenko/fix_freeze

[1.5.0] Fix freeze

lib/network/NetworkConnection.cpp

@@ -12,12 +12,12 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-NetworkConnection::NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, const std::shared_ptr<NetworkContext> & context)
+NetworkConnection::NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket)
 	: socket(socket)
-	, context(context)
 	, listener(listener)
 {
 	socket->set_option(boost::asio::ip::tcp::no_delay(true));
+	socket->set_option(boost::asio::socket_base::keep_alive(true));
 
 	// iOS throws exception on attempt to set buffer size
 	constexpr auto bufferSize = 4 * 1024 * 1024;
@@ -43,32 +43,12 @@ NetworkConnection::NetworkConnection(INetworkConnectionListener & listener, cons
 
 void NetworkConnection::start()
 {
-	heartbeat();
-
 	boost::asio::async_read(*socket,
 							readBuffer,
 							boost::asio::transfer_exactly(messageHeaderSize),
 							[self = shared_from_this()](const auto & ec, const auto & endpoint) { self->onHeaderReceived(ec); });
 }
 
-void NetworkConnection::heartbeat()
-{
-	constexpr auto heartbeatInterval = std::chrono::seconds(10);
-
-	auto timer = std::make_shared<NetworkTimer>(*context, heartbeatInterval);
-	timer->async_wait( [self = shared_from_this(), timer](const auto & ec)
-	{
-		if (ec)
-			return;
-
-		if (!self->socket->is_open())
-			return;
-
-		self->sendPacket({});
-		self->heartbeat();
-	});
-}
-
 void NetworkConnection::onHeaderReceived(const boost::system::error_code & ecHeader)
 {
 	if (ecHeader)
@@ -91,7 +71,7 @@ void NetworkConnection::onHeaderReceived(const boost::system::error_code & ecHea
 
 	if (messageSize == 0)
 	{
-		//heartbeat package with no payload - wait for next packet
+		// Zero-sized packet. Strange, but safe to ignore. Start reading next packet
 		start();
 		return;
 	}
@@ -124,13 +104,16 @@ void NetworkConnection::onPacketReceived(const boost::system::error_code & ec, u
 
 void NetworkConnection::sendPacket(const std::vector<std::byte> & message)
 {
+	std::lock_guard<std::mutex> lock(writeMutex);
+
 	boost::system::error_code ec;
 
 	// create array with single element - boost::asio::buffer can be constructed from containers, but not from plain integer
 	std::array<uint32_t, 1> messageSize{static_cast<uint32_t>(message.size())};
 
 	boost::asio::write(*socket, boost::asio::buffer(messageSize), ec );
-	boost::asio::write(*socket, boost::asio::buffer(message), ec );
+	if (message.size() > 0)
+		boost::asio::write(*socket, boost::asio::buffer(message), ec );
 
 	//Note: ignoring error code, intended
 }

lib/network/NetworkConnection.h

@@ -19,17 +19,16 @@ class NetworkConnection : public INetworkConnection, public std::enable_shared_f
 	static const int messageMaxSize = 64 * 1024 * 1024; // arbitrary size to prevent potential massive allocation if we receive garbage input
 
 	std::shared_ptr<NetworkSocket> socket;
-	std::shared_ptr<NetworkContext> context;
+	std::mutex writeMutex;
 
 	NetworkBuffer readBuffer;
 	INetworkConnectionListener & listener;
 
-	void heartbeat();
 	void onHeaderReceived(const boost::system::error_code & ec);
 	void onPacketReceived(const boost::system::error_code & ec, uint32_t expectedPacketSize);
 
 public:
-	NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket, const std::shared_ptr<NetworkContext> & context);
+	NetworkConnection(INetworkConnectionListener & listener, const std::shared_ptr<NetworkSocket> & socket);
 
 	void start();
 	void close() override;

lib/network/NetworkHandler.cpp

@@ -34,14 +34,14 @@ void NetworkHandler::connectToRemote(INetworkClientListener & listener, const st
 	auto socket = std::make_shared<NetworkSocket>(*io);
 	boost::asio::ip::tcp::resolver resolver(*io);
 	auto endpoints = resolver.resolve(host, std::to_string(port));
-	boost::asio::async_connect(*socket, endpoints, [this, socket, &listener](const boost::system::error_code& error, const boost::asio::ip::tcp::endpoint& endpoint)
+	boost::asio::async_connect(*socket, endpoints, [socket, &listener](const boost::system::error_code& error, const boost::asio::ip::tcp::endpoint& endpoint)
 	{
 		if (error)
 		{
 			listener.onConnectionFailed(error.message());
 			return;
 		}
-		auto connection = std::make_shared<NetworkConnection>(listener, socket, io);
+		auto connection = std::make_shared<NetworkConnection>(listener, socket);
 		connection->start();
 
 		listener.onConnectionEstablished(connection);

lib/network/NetworkServer.cpp

@@ -39,7 +39,7 @@ void NetworkServer::connectionAccepted(std::shared_ptr<NetworkSocket> upcomingCo
 	}
 
 	logNetwork->info("We got a new connection! :)");
-	auto connection = std::make_shared<NetworkConnection>(*this, upcomingConnection, io);
+	auto connection = std::make_shared<NetworkConnection>(*this, upcomingConnection);
 	connections.insert(connection);
 	connection->start();
 	listener.onNewConnection(connection);

lib/networkPacks/NetPacksBase.h

@@ -30,7 +30,7 @@ struct DLL_LINKAGE CPack
 	template <typename Handler> void serialize(Handler &h)
 	{
 		logNetwork->error("CPack serialized... this should not happen!");
-		assert(false && "CPack serialized");
+		throw std::runtime_error("CPack serialized... this should not happen!");
 	}
 
 	void applyGs(CGameState * gs)