Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channel complete close before reconnect #615

Merged
merged 4 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deps/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ else ()

FetchContent_Declare(tlsuv
GIT_REPOSITORY https://github.com/openziti/tlsuv.git
GIT_TAG v0.28.1
GIT_TAG v0.28.2
)
FetchContent_MakeAvailable(tlsuv)

Expand Down
1 change: 1 addition & 0 deletions inc_internal/zt_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ typedef struct ziti_channel {
uint32_t id;
char token[UUID_STR_LEN];
tlsuv_stream_t *connection;
bool reconnect;

// multi purpose timer:
// - reconnect timeout if not connected
Expand Down
52 changes: 31 additions & 21 deletions library/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
static inline void close_connection(ziti_channel_t *ch) {
if (ch->connection) {
tlsuv_stream_t *conn = ch->connection;
ch->connection = NULL;
CH_LOG(DEBUG, "closing TLS[%p]", conn);
tlsuv_stream_close(conn, on_tls_close);
}
}
Expand Down Expand Up @@ -120,13 +120,19 @@
};

static void ch_init_stream(ziti_channel_t *ch) {
if (ch->connection == NULL) {
ch->connection = calloc(1, sizeof(*ch->connection));
tlsuv_stream_init(ch->loop, ch->connection, ch->ctx->tlsCtx);
tlsuv_stream_keepalive(ch->connection, true, 30);
tlsuv_stream_nodelay(ch->connection, true);
ch->connection->data = ch;

if (ch->connection != NULL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ch->connection will be leaked at line 130

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a leftover from debugging. this function should not be called when ch->connection != NULL

ch->connection->data = NULL;
CH_LOG(WARN, "possible stale connection[%p] conn.close_cb[%p]",
ch->connection, ch->connection->close_cb);
}

ch->connection = calloc(1, sizeof(*ch->connection));
tlsuv_stream_init(ch->loop, ch->connection, ch->ctx->tlsCtx);
tlsuv_stream_keepalive(ch->connection, true, 30);
tlsuv_stream_nodelay(ch->connection, true);
ch->connection->data = ch;
ch->reconnect = false;
}

int ziti_channel_prepare(ziti_channel_t *ch) {
Expand Down Expand Up @@ -193,7 +199,7 @@
const char *url;
model_list ch_ids = {0};
MODEL_MAP_FOR(it, ztx->channels) {
model_list_append(&ch_ids, model_map_it_key(it));

Check warning on line 202 in library/channel.c

View workflow job for this annotation

GitHub Actions / Linux ARM64

passing argument 2 of 'model_list_append' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]

Check warning on line 202 in library/channel.c

View workflow job for this annotation

GitHub Actions / Linux x86_64

passing argument 2 of 'model_list_append' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
}

MODEL_LIST_FOR(it, ch_ids) {
Expand All @@ -210,8 +216,15 @@

static void on_tls_close(uv_handle_t *s) {
tlsuv_stream_t *tls = (tlsuv_stream_t *) s;
ziti_channel_t *ch = tls->data;
ch->connection = NULL;
scareything marked this conversation as resolved.
Show resolved Hide resolved

tlsuv_stream_free(tls);
free(tls);

if (ch->reconnect) {
reconnect_channel(ch, true);
}
}

int ziti_channel_close(ziti_channel_t *ch, int err) {
Expand Down Expand Up @@ -739,8 +752,10 @@
if (ztx->api_session == NULL || ztx->api_session->token == NULL || ztx->api_session_state != ZitiApiSessionStateFullyAuthenticated) {
CH_LOG(ERROR, "ziti context is not fully authenticated (api_session_state[%d]), delaying re-connect", ztx->api_session_state);
reconnect_channel(ch, false);
}
else {
} else if (ch->connection != NULL) {
CH_LOG(DEBUG, "connection still closing, deferring reconnect");
ch->reconnect = true;
} else {
ch->msg_seq = 0;

uv_connect_t *req = calloc(1, sizeof(uv_connect_t));
Expand Down Expand Up @@ -864,20 +879,13 @@
tlsuv_stream_read_stop(ssl);
CH_LOG(VERBOSE, "blocked until messages are processed");
return;
case UV_EOF:
case UV_ECONNRESET:
case UV_ECONNABORTED:
case UV_ECONNREFUSED:
case UV_EPIPE:
CH_LOG(INFO, "channel was closed [%zd/%s]", len, uv_strerror(len));

default:
CH_LOG(INFO, "channel disconnected [%zd/%s]", len, uv_strerror(len));
// propagate close
on_channel_close(ch, ZITI_CONNABORT, len);
on_channel_close(ch, ZITI_GATEWAY_UNAVAILABLE, len);
close_connection(ch);
break;

default:
CH_LOG(ERROR, "unhandled error on_data [%zd/%s]", len, uv_strerror(len));
on_channel_close(ch, ZITI_CONNABORT, len);
}
} else if (len == 0) {
// sometimes SSL message has no payload
Expand Down Expand Up @@ -915,7 +923,9 @@
free(r);
}

close_connection(ch);
if (status != UV_ECANCELED) {
close_connection(ch);
}

if (ch->state != Closed) {
ch->state = Disconnected;
Expand Down
Loading