Skip to content

Commit 872c6c6

Browse files
committed
CCBC-1643: fix memory leak of pending buffers in IO context
Change-Id: I946a88cb5e6c1005e2931a83bb437dee87d51f51 Reviewed-on: https://review.couchbase.org/c/libcouchbase/+/213633 Tested-by: Build Bot <[email protected]> Reviewed-by: Sergey Avseyev <[email protected]>
1 parent 39da8ee commit 872c6c6

File tree

2 files changed

+31
-19
lines changed

2 files changed

+31
-19
lines changed

src/mcserver/mcserver.cc

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,7 @@ void Server::handle_connected(lcbio_SOCKET *sock, lcb_STATUS err, lcbio_OSERR sy
10911091
LOGID_T(), lcb_strerror_short(err), syserr, strerror(syserr));
10921092
MC_INCR_METRIC(this, iometrics.io_error, 1);
10931093
if (!maybe_reconnect_on_fake_timeout(err)) {
1094+
lcb_log(LOGARGS_T(DEBUG), LOGFMT "Declare socket failed: %s", LOGID_T(), lcb_strerror_short(err));
10941095
socket_failed(err);
10951096
}
10961097
return;
@@ -1275,6 +1276,18 @@ void Server::close()
12751276
start_errored_ctx(S_CLOSED);
12761277
}
12771278

1279+
/**Marks any unflushed data inside this server as being already flushed. This
1280+
* should be done within error handling. If subsequent data is flushed on this
1281+
* pipeline to the same connection, the results are undefined. */
1282+
void Server::flush_unflushed_data()
1283+
{
1284+
unsigned toflush;
1285+
nb_IOV iov;
1286+
while ((toflush = mcreq_flush_iov_fill(this, &iov, 1, nullptr))) {
1287+
mcreq_flush_done(this, toflush, toflush);
1288+
}
1289+
}
1290+
12781291
/**
12791292
* Call to signal an error or similar on the current socket.
12801293
* @param server The server
@@ -1305,6 +1318,7 @@ void Server::start_errored_ctx(State next_state)
13051318
/* TODO: Maybe throttle reconnection attempts? */
13061319
lcbio_timer_rearm(io_timer, default_timeout());
13071320
}
1321+
flush_unflushed_data();
13081322
connect();
13091323
} else {
13101324
// Connect once someone actually wants a connection.
@@ -1335,30 +1349,26 @@ void Server::start_errored_ctx(State next_state)
13351349
* ctx has pending operations remaining then this function returns immediately.
13361350
* Otherwise this will either reinitialize the connection or free the server
13371351
* object depending on the actual object state (i.e. if it was closed or
1338-
* simply errored).
1352+
* simply errored). The only exception is the case, when the context did not
1353+
* have a chance to be connected, in which case we have to flush everything,
1354+
* close the context and reconnect.
13391355
*/
13401356
void Server::finalize_errored_ctx()
13411357
{
1342-
if (connctx->npending) {
1343-
return;
1344-
}
1345-
1346-
lcb_log(LOGARGS_T(DEBUG), LOGFMT "Finalizing context", LOGID_T());
1347-
1348-
/* Always close the existing context. */
1349-
lcbio_ctx_close(connctx, close_cb, nullptr);
1350-
connctx = nullptr;
1358+
if (connctx != nullptr) {
1359+
if (connreq != nullptr && connctx->npending) {
1360+
return;
1361+
}
13511362

1352-
/**Marks any unflushed data inside this server as being already flushed. This
1353-
* should be done within error handling. If subsequent data is flushed on this
1354-
* pipeline to the same connection, the results are undefined. */
1363+
lcb_log(LOGARGS_T(DEBUG), LOGFMT "Finalizing context", LOGID_T());
13551364

1356-
unsigned toflush;
1357-
nb_IOV iov;
1358-
while ((toflush = mcreq_flush_iov_fill(this, &iov, 1, nullptr))) {
1359-
mcreq_flush_done(this, toflush, toflush);
1365+
/* Always close the existing context. */
1366+
lcbio_ctx_close(connctx, close_cb, nullptr);
1367+
connctx = nullptr;
13601368
}
13611369

1370+
flush_unflushed_data();
1371+
13621372
if (state == Server::S_CLOSED) {
13631373
/* If the server is closed, time to free it */
13641374
delete this;
@@ -1382,8 +1392,8 @@ bool Server::check_closed()
13821392
if (state == Server::S_CLEAN) {
13831393
return false;
13841394
}
1385-
lcb_log(LOGARGS_T(INFO), LOGFMT "Got handler after close. Checking pending calls (pending=%u)", LOGID_T(),
1386-
connctx->npending);
1395+
lcb_log(LOGARGS_T(INFO), LOGFMT "Got handler after close. Checking pending calls (pending=%d)", LOGID_T(),
1396+
(int)(connctx ? connctx->npending : -1));
13871397
finalize_errored_ctx();
13881398
return true;
13891399
}

src/mcserver/mcserver.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ class Server : public mc_PIPELINE
196196
uint32_t next_timeout() const;
197197

198198
bool check_closed();
199+
200+
void flush_unflushed_data();
199201
void start_errored_ctx(State next_state);
200202
void finalize_errored_ctx();
201203
void socket_failed(lcb_STATUS);

0 commit comments

Comments
 (0)