Skip to content

Commit

Permalink
netplay: force safe disposal pattern for IClientConnection:s
Browse files Browse the repository at this point in the history
Mark `IClientConnection` destructor as `protected`, so that
external code cannot accidentally invoke `delete` on a connection
object directly, and thus is forced to call `close()` method on
it for safe disposal.

Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
  • Loading branch information
ManManson committed Feb 24, 2025
1 parent 54aab78 commit c0046b5
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
11 changes: 9 additions & 2 deletions lib/netplay/client_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ class IClientConnection
{
public:

virtual ~IClientConnection() = default;

/// <summary>
/// Read exactly `size` bytes into `buf` buffer.
/// Supports setting a timeout value in milliseconds.
Expand Down Expand Up @@ -219,7 +217,16 @@ class IClientConnection

protected:

// Allow `PendingWritesManager` to access hidden destructor
// since this class ultimately does the final cleanup of
// disposed connections.
friend class PendingWritesManager;

IClientConnection(WzConnectionProvider& connProvider);
// Hide the destructor so that external code cannot accidentally
// `delete` the connection directly and has to use `close()` method
// to dispose of the connection object.
virtual ~IClientConnection() = default;

// Pre-allocated (in ctor) connection list and descriptor sets, which
// only contain `this`.
Expand Down
17 changes: 13 additions & 4 deletions lib/netplay/open_connection_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,25 @@ struct OpenConnectionResult
: open_socket(open_socket)
{ }

public:
bool hasError() const { return errorCode.has_value(); }
public:

OpenConnectionResult(const OpenConnectionResult& other) = delete; // non construction-copyable
OpenConnectionResult& operator=(const OpenConnectionResult&) = delete; // non copyable
OpenConnectionResult(OpenConnectionResult&&) = default;
OpenConnectionResult& operator=(OpenConnectionResult&&) = default;
public:

std::unique_ptr<IClientConnection> open_socket;
struct ConnectionCloser
{
void operator() (IClientConnection* conn)
{
if (conn)
{
conn->close();
}
}
};

std::unique_ptr<IClientConnection, ConnectionCloser> open_socket;
optional<std::error_code> errorCode = nullopt;
std::string errorString;
};
Expand Down

0 comments on commit c0046b5

Please sign in to comment.