Skip to content

Connector internals should always store connection implementation pointers rather than connection objects #140

@CuriousGeorgiy

Description

@CuriousGeorgiy

In our client tests we assume that a destructed connection is automatically closed:

tntcxx/test/ClientTest.cpp

Lines 238 to 262 in a3d0414

template <class BUFFER, class NetProvider>
void
auto_close(Connector<BUFFER, NetProvider> &client)
{
TEST_INIT(0);
{
TEST_CASE("Without requests");
Connection<Buf_t, NetProvider> conn(client);
int rc = test_connect(client, conn, localhost, port);
fail_unless(rc == 0);
}
{
TEST_CASE("With requests");
Connection<Buf_t, NetProvider> conn(client);
int rc = test_connect(client, conn, localhost, port);
fail_unless(rc == 0);
rid_t f = conn.ping();
fail_unless(!conn.futureIsReady(f));
client.wait(conn, f, WAIT_TIMEOUT);
fail_unless(conn.futureIsReady(f));
std::optional<Response<Buf_t>> response = conn.getResponse(f);
fail_unless(response != std::nullopt);
}
}

However, the connection destructor only unreferences the underlying connection implementation rather than closing it:

template<class BUFFER, class NetProvider>
Connection<BUFFER, NetProvider>::~Connection()
{
impl->unref();
}

This is done to make connections copyable. If there were only user connection objects, the connection destructors would eventually trigger connection closure (when the connection implementation has no references left and it gets destroyed). But, currently, the LibevNetProvider uses connection objects internally:

template<class BUFFER, class Stream>
struct WaitWatcher {
WaitWatcher(Connector<BUFFER, LibevNetProvider<BUFFER, Stream>> *client,
Connection<BUFFER, LibevNetProvider<BUFFER, Stream>> conn,
struct ev_timer *t);
struct ev_io in;
struct ev_io out;
Connector<BUFFER, LibevNetProvider<BUFFER, Stream>> *connector;
Connection<BUFFER, LibevNetProvider<BUFFER, Stream>> connection;
struct ev_timer *timer;
};

Rather than using connection implementations like the EpollNetProvider:

template<class BUFFER, class Stream>
void
EpollNetProvider<BUFFER, Stream>::setPollSetting(Conn_t &conn, int setting) {
struct epoll_event event;
event.events = setting;
event.data.ptr = conn.getImpl();

For the LibevNetProvider this means that connections will never get closed automatically, since there will always be an internal connection object.

However, it seems reasonable to rely on the fact that the lifetime of all connections used in the connector internals is tied to the user's connection objects. Hence, we should always store connection implementations rather than connection objects to avoid superfluous referencing.

Metadata

Metadata

Labels

bugSomething isn't working

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions