mirrors
/
OpenTTD
mirror of https://github.com/OpenTTD/OpenTTD.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix #9255: [Network] TCPConnecter crashes when hostname not found (#9259)

This commit is contained in:
Patric Stout 2021-05-13 08:13:48 +02:00 committed by GitHub
parent 38c97e1492
commit d7ce61f106
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 61 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/network/core/tcp.h
View File

@ -66,8 +66,22 @@ public:
*/
class TCPConnecter {
private:
/**
* The current status of the connecter.
*
* We track the status like this to ensure everything is executed from the
* game-thread, and not at another random time where we might not have the
* lock on the game-state.
*/
enum class Status {
INIT, ///< TCPConnecter is created but resolving hasn't started.
RESOLVING, ///< The hostname is being resolved (threaded).
FAILURE, ///< Resolving failed.
CONNECTING, ///< We are currently connecting.
};
std::thread resolve_thread; ///< Thread used during resolving.
std::atomic<bool> is_resolved = false; ///< Whether resolving is done.
std::atomic<Status> status = Status::INIT; ///< The current status of the connecter.
addrinfo *ai = nullptr; ///< getaddrinfo() allocated linked-list of resolved addresses.
std::vector<addrinfo *> addresses; ///< Addresses we can connect to.

55
src/network/core/tcp_connect.cpp
View File

@ -31,10 +31,6 @@ TCPConnecter::TCPConnecter(const std::string &connection_string, uint16 default_
this->connection_string = NormalizeConnectionString(connection_string, default_port);
_tcp_connecters.push_back(this);
if (!StartNewThread(&this->resolve_thread, "ottd:resolve", &TCPConnecter::ResolveThunk, this)) {
this->Resolve();
}
}
TCPConnecter::~TCPConnecter()
@ -100,6 +96,10 @@ bool TCPConnecter::TryNextAddress()
return true;
}
/**
* Callback when resolving is done.
* @param ai A linked-list of address information.
*/
void TCPConnecter::OnResolved(addrinfo *ai)
{
std::deque<addrinfo *> addresses_ipv4, addresses_ipv6;
@ -159,6 +159,12 @@ void TCPConnecter::OnResolved(addrinfo *ai)
this->current_address = 0;
}
/**
* Start resolving the hostname.
*
* This function must change "status" to either Status::FAILURE
* or Status::CONNECTING before returning.
*/
void TCPConnecter::Resolve()
{
/* Port is already guaranteed part of the connection_string. */
@ -177,7 +183,7 @@ void TCPConnecter::Resolve()
auto start = std::chrono::steady_clock::now();
addrinfo *ai;
int e = getaddrinfo(address.GetHostname(), port_name, &hints, &ai);
int error = getaddrinfo(address.GetHostname(), port_name, &hints, &ai);
auto end = std::chrono::steady_clock::now();
auto duration = std::chrono::duration_cast<std::chrono::seconds>(end - start);
@ -187,18 +193,21 @@ void TCPConnecter::Resolve()
getaddrinfo_timeout_error_shown = true;
}
if (e != 0) {
if (error != 0) {
DEBUG(net, 0, "Failed to resolve DNS for %s", this->connection_string.c_str());
this->OnFailure();
this->status = Status::FAILURE;
return;
}
this->ai = ai;
this->OnResolved(ai);
this->is_resolved = true;
this->status = Status::CONNECTING;
}
/**
* Thunk to start Resolve() on the right instance.
*/
/* static */ void TCPConnecter::ResolveThunk(TCPConnecter *connecter)
{
connecter->Resolve();
@ -210,7 +219,35 @@ void TCPConnecter::Resolve()
*/
bool TCPConnecter::CheckActivity()
{
if (!this->is_resolved.load()) return false;
switch (this->status.load()) {
case Status::INIT:
/* Start the thread delayed, so the vtable is loaded. This allows classes
* to overload functions used by Resolve() (in case threading is disabled). */
if (StartNewThread(&this->resolve_thread, "ottd:resolve", &TCPConnecter::ResolveThunk, this)) {
this->status = Status::RESOLVING;
return false;
}
/* No threads, do a blocking resolve. */
this->Resolve();
/* Continue as we are either failed or can start the first
* connection. The rest of this function handles exactly that. */
break;
case Status::RESOLVING:
/* Wait till Resolve() comes back with an answer (in case it runs threaded). */
return false;
case Status::FAILURE:
/* Ensure the OnFailure() is called from the game-thread instead of the
* resolve-thread, as otherwise we can get into some threading issues. */
this->OnFailure();
return true;
case Status::CONNECTING:
break;
}
/* If there are no attempts pending, connect to the next. */
if (this->sockets.empty()) {