Disable srvdst events asap when xferring to child and in split mode

This is the correct implementation. And disabling later on is
problematic while terminating, and can even cause a crash.
This commit is contained in:
Soner Tari 2022-04-06 19:41:59 +03:00
parent 61e28a5c86
commit 722c2f59d2
5 changed files with 24 additions and 30 deletions

View File

@ -84,16 +84,6 @@ protopassthrough_engage(pxy_conn_ctx_t *ctx)
// If srvdst has been xferred to the first child conn, the child should free it, not the parent
if (ctx->divert && !ctx->srvdst_xferred) {
ctx->srvdst.free(ctx->srvdst.bev, ctx);
} else /*if (!ctx->divert || ctx->srvdst_xferred)*/ {
struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev);
bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL);
bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE);
if (ubev) {
bufferevent_setcb(ubev, NULL, NULL, NULL, NULL);
bufferevent_disable(ubev, EV_READ|EV_WRITE);
}
}
ctx->srvdst.bev = NULL;
ctx->srvdst.ssl = NULL;

View File

@ -1533,6 +1533,10 @@ protossl_setup_dst_child(pxy_conn_child_ctx_t *ctx)
ctx->conn->srvdst_xferred = 1;
ctx->srvdst_xferred = 1;
ctx->dst = ctx->conn->srvdst;
// See the comments in prototcp_setup_dst()
prototcp_disable_events_srvdst(ctx->conn);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb_child, pxy_bev_writecb_child, pxy_bev_eventcb_child, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
} else {

View File

@ -111,6 +111,19 @@ prototcp_setup_src(pxy_conn_ctx_t *ctx)
return 0;
}
void
prototcp_disable_events_srvdst(pxy_conn_ctx_t *ctx)
{
bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL);
bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE);
struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev);
if (ubev) {
bufferevent_setcb(ubev, NULL, NULL, NULL, NULL);
bufferevent_disable(ubev, EV_READ|EV_WRITE);
}
}
int
prototcp_setup_dst(pxy_conn_ctx_t *ctx)
{
@ -131,17 +144,12 @@ prototcp_setup_dst(pxy_conn_ctx_t *ctx)
// But if we don't NULL the callbacks of srvdst in split mode,
// we randomly but rarely get a second eof event for srvdst during conn termination (especially on arm64),
// which crashes us with signal 11 or 10, because the first eof event for dst frees the ctx.
// This does not seem to happen with srvdst_xferred, but just to be safe we do the same for it too.
// Note that we don't free anything here, but just disable callbacks and events.
// This seems to be an issue with libevent.
// @todo Why does libevent raise the same event again for an already disabled and freed conn end?
// Note again that srvdst == dst or child_dst here.
bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL);
bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE);
struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev);
if (ubev) {
bufferevent_setcb(ubev, NULL, NULL, NULL, NULL);
bufferevent_disable(ubev, EV_READ|EV_WRITE);
}
prototcp_disable_events_srvdst(ctx);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb, pxy_bev_writecb, pxy_bev_eventcb, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
@ -200,6 +208,10 @@ prototcp_setup_dst_child(pxy_conn_child_ctx_t *ctx)
ctx->conn->srvdst_xferred = 1;
ctx->srvdst_xferred = 1;
ctx->dst = ctx->conn->srvdst;
// See the comments in prototcp_setup_dst()
prototcp_disable_events_srvdst(ctx->conn);
bufferevent_setcb(ctx->dst.bev, pxy_bev_readcb_child, pxy_bev_writecb_child, pxy_bev_eventcb_child, ctx);
ctx->protoctx->bev_eventcb(ctx->dst.bev, BEV_EVENT_CONNECTED, ctx);
} else {

View File

@ -82,6 +82,7 @@ int prototcp_enable_src(pxy_conn_ctx_t *) NONNULL(1);
void prototcp_bev_eventcb_srvdst(struct bufferevent *, short, pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_src(pxy_conn_ctx_t *) NONNULL(1);
void prototcp_disable_events_srvdst(pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_dst(pxy_conn_ctx_t *) NONNULL(1);
int prototcp_setup_srvdst(pxy_conn_ctx_t *) NONNULL(1);

View File

@ -416,20 +416,7 @@ pxy_conn_free(pxy_conn_ctx_t *ctx, int by_requestor)
// If srvdst has been xferred to the first child conn, the child should free it, not the parent
if (ctx->divert && !ctx->srvdst_xferred) {
ctx->srvdst.free(ctx->srvdst.bev, ctx);
} else if (ctx->srvdst_xferred) {
// @see prototcp_setup_dst()
// It does not seem to happen with srvdst_xferred, but just to be safe we do the same for it too.
struct bufferevent *ubev = bufferevent_get_underlying(ctx->srvdst.bev);
bufferevent_setcb(ctx->srvdst.bev, NULL, NULL, NULL, NULL);
bufferevent_disable(ctx->srvdst.bev, EV_READ|EV_WRITE);
if (ubev) {
bufferevent_setcb(ubev, NULL, NULL, NULL, NULL);
bufferevent_disable(ubev, EV_READ|EV_WRITE);
}
}
// else if (!ctx->divert) {}
ctx->srvdst.bev = NULL;
}