From: Oto Šťáva Date: Fri, 10 Mar 2023 12:15:16 +0000 (+0100) Subject: daemon: improve session closure readability X-Git-Tag: v6.0.2~42^2~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9d6cc0bf7f8409db06ef2d0ff61c55bfb496daa8;p=thirdparty%2Fknot-resolver.git daemon: improve session closure readability Until now, sessions were closed by explicitly sending `_CLOSE` events via the `session2_event()` function to them, which I think is not signalling the intent very well. It might look as though the session has been/is being closed by some part of the code that contains the `session2_event()` call and a relevant event is being fired now. This commit introduces `session2_close()` and `session2_force_close()` inline functions, which do the same thing, but I think the intent behind calling them is slightly clearer. --- diff --git a/daemon/io.c b/daemon/io.c index 485f5b29e..1014c046d 100644 --- a/daemon/io.c +++ b/daemon/io.c @@ -309,7 +309,7 @@ static enum protolayer_iter_cb_result pl_tcp_unwrap( worker_end_tcp(s); return protolayer_break(ctx, kr_error(ECONNRESET)); } else if (trimmed == 0) { - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); return protolayer_break(ctx, kr_error(ECONNRESET)); } @@ -552,7 +552,7 @@ static void _tcp_accept(uv_stream_t *master, int status, enum protolayer_grp grp if (uv_accept(master, (uv_stream_t *)client) != 0) { /* close session, close underlying uv handles and * deallocate (or return to memory pool) memory. */ - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); return; } @@ -562,14 +562,14 @@ static void _tcp_accept(uv_stream_t *master, int status, enum protolayer_grp grp int sa_len = sizeof(struct sockaddr_in6); int ret = uv_tcp_getpeername(client, sa, &sa_len); if (ret || sa->sa_family == AF_UNSPEC) { - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); return; } sa = session2_get_sockname(s); sa_len = sizeof(struct sockaddr_in6); ret = uv_tcp_getsockname(client, sa, &sa_len); if (ret || sa->sa_family == AF_UNSPEC) { - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); return; } diff --git a/daemon/network.c b/daemon/network.c index 75a517f29..1ec34e907 100644 --- a/daemon/network.c +++ b/daemon/network.c @@ -234,11 +234,11 @@ static void endpoint_close(struct endpoint *ep, bool force) ep->handle->loop = NULL; struct session2 *s = ep->handle->data; if (s) - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); } } else { /* Asynchronous close */ struct session2 *s = ep->handle->data; - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); } } diff --git a/daemon/session2.c b/daemon/session2.c index 2bba4a220..2f112b189 100644 --- a/daemon/session2.c +++ b/daemon/session2.c @@ -1537,5 +1537,5 @@ void session2_kill_ioreq(struct session2 *session, struct qr_task *task) return; session2_tasklist_del(session, task); if (session->transport.io.handle->type == UV_UDP) - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); } diff --git a/daemon/session2.h b/daemon/session2.h index 04e090675..a0d6701f5 100644 --- a/daemon/session2.h +++ b/daemon/session2.h @@ -326,13 +326,12 @@ typedef void (*protolayer_finished_cb)(int status, struct session2 *session, * Event types are used to distinguish different events that can be passed to * sessions using `session2_event()`. */ #define PROTOLAYER_EVENT_MAP(XX) \ - XX(CLOSE) /**< Signal to gracefully close the session - + XX(CLOSE) /**< Sending this event closes the session gracefully - * i.e. layers add their standard disconnection * ceremony (e.g. `gnutls_bye()`). */\ - XX(FORCE_CLOSE) /**< Signal to forcefully close the - * session - i.e. layers SHOULD NOT add - * any disconnection ceremony, if - * avoidable. */\ + XX(FORCE_CLOSE) /**< Sending this event closes the session forcefully - + * i.e. layers SHOULD NOT add any disconnection + * ceremony, if avoidable. */\ XX(CONNECT_TIMEOUT) /**< Signal that a connection could not be * established due to a timeout. */\ XX(GENERAL_TIMEOUT) /**< Signal that a general application-defined @@ -1066,6 +1065,24 @@ void session2_event(struct session2 *s, enum protolayer_event_type event, void * void session2_event_after(struct session2 *s, enum protolayer_protocol protocol, enum protolayer_event_type event, void *baton); +/** Sends a `PROTOLAYER_EVENT_CLOSE` event to be processed by the protocol + * layers of the specified session. This function exists for readability + * reasons, to signal the intent that sending this event is used to actually + * close the session. */ +static inline void session2_close(struct session2 *s) +{ + session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); +} + +/** Sends a `PROTOLAYER_EVENT_FORCE_CLOSE` event to be processed by the + * protocol layers of the specified session. This function exists for + * readability reasons, to signal the intent that sending this event is used to + * actually close the session. */ +static inline void session2_force_close(struct session2 *s) +{ + session2_event(s, PROTOLAYER_EVENT_FORCE_CLOSE, NULL); +} + /** Performs initial setup of the specified `req`, using the session's protocol * layers. Layers are processed in the `_UNWRAP` direction. */ void session2_init_request(struct session2 *s, struct kr_request *req); diff --git a/daemon/worker.c b/daemon/worker.c index e529e5b91..3805638df 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -158,7 +158,7 @@ static struct session2 *ioreq_spawn(int socktype, sa_family_t family, } if (addr->ip.sa_family != AF_UNSPEC) { if (kr_fails_assert(addr->ip.sa_family == family)) { - session2_event(s, PROTOLAYER_EVENT_FORCE_CLOSE, NULL); + session2_force_close(s); return NULL; } if (socktype == SOCK_DGRAM) { @@ -171,7 +171,7 @@ static struct session2 *ioreq_spawn(int socktype, sa_family_t family, } if (ret != 0) { - session2_event(s, PROTOLAYER_EVENT_FORCE_CLOSE, NULL); + session2_force_close(s); return NULL; } @@ -717,7 +717,7 @@ static int send_waiting(struct session2 *session) session2_waitinglist_finalize(session, KR_STATE_FAIL); session2_tasklist_finalize(session, KR_STATE_FAIL); worker_del_tcp_connected(peer); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); break; } session2_waitinglist_pop(session, true); @@ -760,7 +760,7 @@ static void on_connect(uv_connect_t *req, int status) } kr_assert(session2_tasklist_is_empty(session)); session2_waitinglist_retry(session, false); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return; } @@ -777,7 +777,7 @@ static void on_connect(uv_connect_t *req, int status) } kr_assert(session2_tasklist_is_empty(session)); session2_waitinglist_retry(session, false); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return; } @@ -794,7 +794,7 @@ static void on_connect(uv_connect_t *req, int status) session2_event(session, PROTOLAYER_EVENT_CONNECT_FAIL, NULL); } kr_assert(session2_tasklist_is_empty(session)); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return; } @@ -844,7 +844,7 @@ static int transmit(struct qr_task *task) }; ret = qr_task_send(task, session, &out_comm, task->pktbuf); if (ret) { - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return ret; } @@ -993,7 +993,7 @@ static int qr_task_finalize(struct qr_task *task, int state) * (ie. task->leading is true) */ worker_task_unref(t); } - session2_event(source_session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(source_session); } qr_task_unref(task); @@ -1063,7 +1063,7 @@ static int tcp_task_existing_connection(struct session2 *session, struct qr_task * close connection to upstream. */ session2_tasklist_finalize(session, KR_STATE_FAIL); worker_del_tcp_connected(session2_get_peer(session)); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return kr_error(EINVAL); } @@ -1108,7 +1108,7 @@ static int tcp_task_make_connection(struct qr_task *task, const struct sockaddr int ret = worker_add_tcp_waiting(addr, session); if (ret < 0) { free(conn); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return kr_error(EINVAL); } @@ -1123,7 +1123,7 @@ static int tcp_task_make_connection(struct qr_task *task, const struct sockaddr if (ret != 0) { worker_del_tcp_waiting(addr); free(conn); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return kr_error(EINVAL); } @@ -1140,7 +1140,7 @@ static int tcp_task_make_connection(struct qr_task *task, const struct sockaddr session2_timer_stop(session); worker_del_tcp_waiting(addr); free(conn); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); qry->server_selection.error(qry, task->transport, KR_SELECTION_TCP_CONNECT_FAILED); return kr_error(EAGAIN); } @@ -1152,7 +1152,7 @@ static int tcp_task_make_connection(struct qr_task *task, const struct sockaddr session2_timer_stop(session); worker_del_tcp_waiting(addr); free(conn); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return kr_error(EINVAL); } @@ -1490,7 +1490,7 @@ int worker_end_tcp(struct session2 *session) return kr_error(EINVAL); session2_timer_stop(session); - session2_event(session, PROTOLAYER_EVENT_FORCE_CLOSE, NULL); + session2_force_close(session); return kr_ok(); } @@ -1889,7 +1889,7 @@ static enum protolayer_event_cb_result pl_dns_stream_resolution_timeout( worker_del_tcp_waiting(peer); worker_del_tcp_connected(peer); } - session2_event(s, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(s); } } @@ -1910,7 +1910,7 @@ static enum protolayer_event_cb_result pl_dns_stream_connected( * something gone wrong */ session2_waitinglist_finalize(session, KR_STATE_FAIL); kr_assert(session2_tasklist_is_empty(session)); - session2_event(session, PROTOLAYER_EVENT_CLOSE, NULL); + session2_close(session); return PROTOLAYER_EVENT_CONSUME; }