]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
daemon: improve session closure readability
authorOto Šťáva <oto.stava@nic.cz>
Fri, 10 Mar 2023 12:15:16 +0000 (13:15 +0100)
committerOto Šťáva <oto.stava@nic.cz>
Fri, 17 Mar 2023 07:18:43 +0000 (08:18 +0100)
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.

daemon/io.c
daemon/network.c
daemon/session2.c
daemon/session2.h
daemon/worker.c

index 485f5b29e4f8c7ca95ea7997d1f6e6cd1ed4849d..1014c046d8865aafb9629ae46d615542b9e4bd7c 100644 (file)
@@ -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;
        }
 
index 75a517f29f63f1b55583f190714f11b03745f11c..1ec34e9070fc237be7d774ecdc392415268c3592 100644 (file)
@@ -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);
        }
 }
 
index 2bba4a220a31a96eb938529ca8520f5a3da41188..2f112b189d38d2e26299c31991eb153a7fba0333 100644 (file)
@@ -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);
 }
index 04e090675dd9aa6344f7240d00515fae8898e7ef..a0d6701f5c0684ef9bd33b3f9ddbd67eb979576d 100644 (file)
@@ -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);
index e529e5b917f978559a7be9e3e47f99d9ae3f3078..3805638df687c292d2dad96d2ddda26d65726257 100644 (file)
@@ -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;
        }