From d330dbd76a5208be253b3b4ecbf8af312ba4880a Mon Sep 17 00:00:00 2001 From: Andy Green Date: Thu, 24 Dec 2020 16:06:50 +0000 Subject: [PATCH] wsi: unify base wsi creation function A few different places want to create wsis and basically repeat their own versions of the flow. Let's unify it into one helper in wsi.c Also require the context lock held (this only impacts LWS_MAX_SMP > 1) --- lib/core-net/adopt.c | 23 ++-------- lib/core-net/client/connect.c | 66 ++++++++++++----------------- lib/core-net/client/connect3.c | 6 +-- lib/core-net/private-lib-core-net.h | 2 +- lib/core-net/vhost.c | 4 +- lib/core-net/wsi.c | 13 +++++- lib/core/context.c | 5 ++- lib/core/private-lib-core.h | 4 +- lib/plat/unix/unix-spawn.c | 28 +++--------- lib/plat/windows/windows-spawn.c | 16 ++----- lib/roles/dbus/dbus.c | 24 ++++------- lib/roles/http/server/server.c | 27 ++++-------- lib/roles/netlink/ops-netlink.c | 5 ++- 13 files changed, 87 insertions(+), 136 deletions(-) diff --git a/lib/core-net/adopt.c b/lib/core-net/adopt.c index 198d1cb7a..2414f32bd 100644 --- a/lib/core-net/adopt.c +++ b/lib/core-net/adopt.c @@ -47,10 +47,8 @@ lws_get_idlest_tsi(struct lws_context *context) struct lws * lws_create_new_server_wsi(struct lws_vhost *vhost, int fixed_tsi) { - struct lws_context_per_thread *pt; struct lws *new_wsi; int n = fixed_tsi; - size_t s = sizeof(struct lws); if (n < 0) n = lws_get_idlest_tsi(vhost->context); @@ -60,29 +58,20 @@ lws_create_new_server_wsi(struct lws_vhost *vhost, int fixed_tsi) return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - s += vhost->context->event_loop_ops->evlib_size_wsi; -#endif - - new_wsi = lws_zalloc(s, "new server wsi"); + lws_context_lock(vhost->context, __func__); + new_wsi = __lws_wsi_create_with_role(vhost->context, fixed_tsi, NULL); + lws_context_unlock(vhost->context); if (new_wsi == NULL) { lwsl_err("Out of memory for new connection\n"); return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - new_wsi->evlib_wsi = (uint8_t *)new_wsi + sizeof(*new_wsi); -#endif - new_wsi->wsistate |= LWSIFR_SERVER; new_wsi->tsi = n; - pt = &vhost->context->pt[n]; lwsl_debug("new wsi %p joining vhost %s, tsi %d\n", new_wsi, vhost->name, new_wsi->tsi); lws_vhost_bind_wsi(vhost, new_wsi); - new_wsi->a.context = vhost->context; - new_wsi->pending_timeout = NO_PENDING_TIMEOUT; new_wsi->rxflow_change_to = LWS_RXFLOW_ALLOW; new_wsi->retry_policy = vhost->retry_policy; @@ -108,12 +97,6 @@ lws_create_new_server_wsi(struct lws_vhost *vhost, int fixed_tsi) */ new_wsi->a.protocol = vhost->protocols; new_wsi->user_space = NULL; - new_wsi->desc.sockfd = LWS_SOCK_INVALID; - new_wsi->position_in_fds_table = LWS_NO_FDS_POS; - - lws_pt_lock(pt, __func__); - pt->count_wsi_allocated++; - lws_pt_unlock(pt); /* * outermost create notification for wsi diff --git a/lib/core-net/client/connect.c b/lib/core-net/client/connect.c index 0a624c8c8..05c901e98 100644 --- a/lib/core-net/client/connect.c +++ b/lib/core-net/client/connect.c @@ -90,9 +90,8 @@ lws_client_connect_via_info(const struct lws_client_connect_info *i) const char *local = i->protocol; struct lws *wsi, *safe = NULL; const struct lws_protocols *p; - size_t s = sizeof(struct lws); const char *cisin[CIS_COUNT]; - int tid = 0, n, m; + int tid = 0, n, m, tsi = 0; size_t size; char *pc; @@ -112,18 +111,39 @@ lws_client_connect_via_info(const struct lws_client_connect_info *i) lws_stats_bump(&i->context->pt[tid], LWSSTATS_C_CONNS_CLIENT, 1); - /* PHASE 1: create a bare wsi */ -#if defined(LWS_WITH_EVENT_LIBS) - s += i->context->event_loop_ops->evlib_size_wsi; + lws_context_lock(i->context, __func__); + /* + * PHASE 1: if SMP, find out the tsi related to current service thread + */ + +#if LWS_MAX_SMP > 1 + + for (n = 0; n < i->context->count_threads; n++) + if (i->context->pt[n].service_tid == tid) { + lwsl_info("%s: client binds to caller tsi %d\n", + __func__, n); + tsi = n; + + break; + } + + /* + * this binding is sort of provisional, since when we try to insert + * into the pt fds, there may be no space and it will fail + */ + #endif - wsi = lws_zalloc(s, "client wsi"); + /* PHASE 2: create a bare wsi */ + + wsi = __lws_wsi_create_with_role(i->context, tsi, NULL); + lws_context_unlock(i->context); if (wsi == NULL) goto bail; -#if defined(LWS_WITH_EVENT_LIBS) - wsi->evlib_wsi = (uint8_t *)wsi + sizeof(*wsi); +#if defined(LWS_WITH_DETAILED_LATENCY) && LWS_MAX_SMP > 1 + wsi->detlat.tsi = tsi; #endif /* @@ -137,8 +157,6 @@ lws_client_connect_via_info(const struct lws_client_connect_info *i) else wsi->keep_warm_secs = 5; - wsi->a.context = i->context; - wsi->desc.sockfd = LWS_SOCK_INVALID; wsi->seq = i->seq; wsi->flags = i->ssl_connection; if (i->retry_and_idle_policy) @@ -154,7 +172,6 @@ lws_client_connect_via_info(const struct lws_client_connect_info *i) if (i->ssl_connection & LCCSCF_WAKE_SUSPEND__VALIDITY) wsi->conn_validity_wakesuspend = 1; - wsi->a.vhost = NULL; if (!i->vhost) { struct lws_vhost *v = i->context->vhost_list; @@ -179,33 +196,6 @@ lws_client_connect_via_info(const struct lws_client_connect_info *i) LWS_CALLBACK_GET_THREAD_ID, NULL, NULL, 0); #endif - /* - * PHASE 2: if SMP, bind the client to whatever tsi the current thread - * represents - */ - -#if LWS_MAX_SMP > 1 - lws_context_lock(i->context, "client find tsi"); - - for (n = 0; n < i->context->count_threads; n++) - if (i->context->pt[n].service_tid == tid) { - lwsl_info("%s: client binds to caller tsi %d\n", - __func__, n); - wsi->tsi = n; -#if defined(LWS_WITH_DETAILED_LATENCY) - wsi->detlat.tsi = n; -#endif - break; - } - - /* - * this binding is sort of provisional, since when we try to insert - * into the pt fds, there may be no space and it will fail - */ - - lws_context_unlock(i->context); -#endif - /* * PHASE 3: Choose an initial role for the wsi and do role-specific init * diff --git a/lib/core-net/client/connect3.c b/lib/core-net/client/connect3.c index bd1975e91..f872917e0 100644 --- a/lib/core-net/client/connect3.c +++ b/lib/core-net/client/connect3.c @@ -524,9 +524,9 @@ oom4: struct lws_vhost *vhost = wsi->a.vhost; lws_sockfd_type sfd = wsi->desc.sockfd; - lws_vhost_lock(vhost); - __lws_free_wsi(wsi); - lws_vhost_unlock(vhost); + //lws_vhost_lock(vhost); + __lws_free_wsi(wsi); /* acquires vhost lock in wsi reset */ + //lws_vhost_unlock(vhost); sanity_assert_no_wsi_traces(vhost->context, wsi); sanity_assert_no_sockfd_traces(vhost->context, sfd); diff --git a/lib/core-net/private-lib-core-net.h b/lib/core-net/private-lib-core-net.h index 5f225587b..df947ba95 100644 --- a/lib/core-net/private-lib-core-net.h +++ b/lib/core-net/private-lib-core-net.h @@ -1250,7 +1250,7 @@ lws_http_client_connect_via_info2(struct lws *wsi); struct lws * -lws_wsi_create_with_role(struct lws_context *context, int tsi, +__lws_wsi_create_with_role(struct lws_context *context, int tsi, const struct lws_role_ops *ops); int lws_wsi_inject_to_loop(struct lws_context_per_thread *pt, struct lws *wsi); diff --git a/lib/core-net/vhost.c b/lib/core-net/vhost.c index f8a61c72a..62cf20c6f 100644 --- a/lib/core-net/vhost.c +++ b/lib/core-net/vhost.c @@ -920,7 +920,7 @@ lws_cancel_service(struct lws_context *context) } int -lws_create_event_pipes(struct lws_context *context) +__lws_create_event_pipes(struct lws_context *context) { struct lws_context_per_thread *pt; struct lws *wsi; @@ -942,7 +942,7 @@ lws_create_event_pipes(struct lws_context *context) if (pt->pipe_wsi) return 0; - wsi = lws_wsi_create_with_role(context, n, &role_ops_pipe); + wsi = __lws_wsi_create_with_role(context, n, &role_ops_pipe); if (!wsi) return 1; diff --git a/lib/core-net/wsi.c b/lib/core-net/wsi.c index c33b72da0..75a8f0643 100644 --- a/lib/core-net/wsi.c +++ b/lib/core-net/wsi.c @@ -190,14 +190,20 @@ lws_callback_vhost_protocols(struct lws *wsi, int reason, void *in, int len) return 0; } +/* + * We need the context lock + */ + struct lws * -lws_wsi_create_with_role(struct lws_context *context, int tsi, +__lws_wsi_create_with_role(struct lws_context *context, int tsi, const struct lws_role_ops *ops) { struct lws_context_per_thread *pt = &context->pt[tsi]; size_t s = sizeof(struct lws); struct lws *wsi; + lws_context_assert_lock_held(context); + #if defined(LWS_WITH_EVENT_LIBS) s += context->event_loop_ops->evlib_size_wsi; #endif @@ -214,15 +220,20 @@ lws_wsi_create_with_role(struct lws_context *context, int tsi, #endif wsi->a.context = context; lws_role_transition(wsi, 0, LRS_UNCONNECTED, ops); + wsi->pending_timeout = NO_PENDING_TIMEOUT; wsi->a.protocol = NULL; wsi->tsi = tsi; wsi->a.vhost = NULL; wsi->desc.sockfd = LWS_SOCK_INVALID; + wsi->position_in_fds_table = LWS_NO_FDS_POS; lws_pt_lock(pt, __func__); pt->count_wsi_allocated++; lws_pt_unlock(pt); + lwsl_notice("%s: tsi %d: role: %s\n", __func__, tsi, + ops ? ops->name : "none"); + return wsi; } diff --git a/lib/core/context.c b/lib/core/context.c index 893bdfb21..3e74a5c89 100644 --- a/lib/core/context.c +++ b/lib/core/context.c @@ -1068,7 +1068,10 @@ lws_create_context(const struct lws_context_creation_info *info) goto bail_libuv_aware; } - if (lws_create_event_pipes(context)) + lws_context_lock(context, __func__); + n = __lws_create_event_pipes(context); + lws_context_unlock(context); + if (n) goto bail_libuv_aware; for (n = 0; n < context->count_threads; n++) { diff --git a/lib/core/private-lib-core.h b/lib/core/private-lib-core.h index ecb009d42..2c598b873 100644 --- a/lib/core/private-lib-core.h +++ b/lib/core/private-lib-core.h @@ -637,6 +637,7 @@ void lwsl_emit_stderr(int level, const char *line); #if LWS_MAX_SMP > 1 #define lws_context_lock(c, reason) lws_mutex_refcount_lock(&c->mr, reason) #define lws_context_unlock(c) lws_mutex_refcount_unlock(&c->mr) +#define lws_context_assert_lock_held(c) lws_mutex_refcount_assert_held(&c->mr) static LWS_INLINE void lws_vhost_lock(struct lws_vhost *vhost) @@ -659,6 +660,7 @@ lws_vhost_unlock(struct lws_vhost *vhost) #define lws_pt_unlock(_a) (void)(_a) #define lws_context_lock(_a, _b) (void)(_a) #define lws_context_unlock(_a) (void)(_a) +#define lws_context_assert_lock_held(_a) (void)(_a) #define lws_vhost_lock(_a) (void)(_a) #define lws_vhost_unlock(_a) (void)(_a) #define lws_pt_stats_lock(_a) (void)(_a) @@ -713,7 +715,7 @@ void lws_free(void *p); #endif int -lws_create_event_pipes(struct lws_context *context); +__lws_create_event_pipes(struct lws_context *context); int lws_plat_apply_FD_CLOEXEC(int n); diff --git a/lib/plat/unix/unix-spawn.c b/lib/plat/unix/unix-spawn.c index 8277122a6..28c304e02 100644 --- a/lib/plat/unix/unix-spawn.c +++ b/lib/plat/unix/unix-spawn.c @@ -63,39 +63,28 @@ lws_spawn_sul_reap(struct lws_sorted_usec_list *sul) } static struct lws * -lws_create_basic_wsi(struct lws_context *context, int tsi, +lws_create_stdwsi(struct lws_context *context, int tsi, const struct lws_role_ops *ops) { struct lws_context_per_thread *pt = &context->pt[tsi]; - size_t s = sizeof(struct lws); struct lws *new_wsi; if (!context->vhost_list) return NULL; - if ((unsigned int)context->pt[tsi].fds_count == - context->fd_limit_per_thread - 1) { + if ((unsigned int)pt->fds_count == context->fd_limit_per_thread - 1) { lwsl_err("no space for new conn\n"); return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - s += context->event_loop_ops->evlib_size_wsi; -#endif - - new_wsi = lws_zalloc(s, "new wsi"); + lws_context_lock(context, __func__); + new_wsi = __lws_wsi_create_with_role(context, tsi, ops); + lws_context_unlock(context); if (new_wsi == NULL) { lwsl_err("Out of memory for new connection\n"); return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - new_wsi->evlib_wsi = (uint8_t *)new_wsi + sizeof(*new_wsi); -#endif - - new_wsi->tsi = tsi; - new_wsi->a.context = context; - new_wsi->pending_timeout = NO_PENDING_TIMEOUT; new_wsi->rxflow_change_to = LWS_RXFLOW_ALLOW; /* initialize the instance struct */ @@ -103,7 +92,6 @@ lws_create_basic_wsi(struct lws_context *context, int tsi, lws_role_transition(new_wsi, 0, LRS_ESTABLISHED, ops); new_wsi->hdr_parsing_completed = 0; - new_wsi->position_in_fds_table = LWS_NO_FDS_POS; /* * these can only be set once the protocol is known @@ -113,10 +101,6 @@ lws_create_basic_wsi(struct lws_context *context, int tsi, */ new_wsi->user_space = NULL; - new_wsi->desc.sockfd = LWS_SOCK_INVALID; - lws_pt_lock(pt, __func__); - pt->count_wsi_allocated++; - lws_pt_unlock(pt); return new_wsi; } @@ -365,7 +349,7 @@ lws_spawn_piped(const struct lws_spawn_piped_info *i) /* create wsis for each stdin/out/err fd */ for (n = 0; n < 3; n++) { - lsp->stdwsi[n] = lws_create_basic_wsi(i->vh->context, i->tsi, + lsp->stdwsi[n] = lws_create_stdwsi(i->vh->context, i->tsi, i->ops ? i->ops : &role_ops_raw_file); if (!lsp->stdwsi[n]) { lwsl_err("%s: unable to create lsp stdwsi\n", __func__); diff --git a/lib/plat/windows/windows-spawn.c b/lib/plat/windows/windows-spawn.c index 7ac078005..427fa5d9c 100644 --- a/lib/plat/windows/windows-spawn.c +++ b/lib/plat/windows/windows-spawn.c @@ -67,7 +67,6 @@ lws_create_basic_wsi(struct lws_context *context, int tsi, { struct lws_context_per_thread *pt = &context->pt[tsi]; struct lws *new_wsi; - size_t s = sizeof(*new_wsi); if (!context->vhost_list) return NULL; @@ -78,23 +77,14 @@ lws_create_basic_wsi(struct lws_context *context, int tsi, return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - s += vhost->context->event_loop_ops->evlib_size_wsi; -#endif - - new_wsi = lws_zalloc(s, "new wsi"); + lws_context_lock(context, __func__); + new_wsi = __lws_wsi_create_with_role(context, tsi, ops); + lws_context_unlock(context); if (new_wsi == NULL) { lwsl_err("Out of memory for new connection\n"); return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - new_wsi->evlib_wsi = (uint8_t *)new_wsi + sizeof(*new_wsi); -#endif - - new_wsi->tsi = tsi; - new_wsi->a.context = context; - new_wsi->pending_timeout = NO_PENDING_TIMEOUT; new_wsi->rxflow_change_to = LWS_RXFLOW_ALLOW; /* initialize the instance struct */ diff --git a/lib/roles/dbus/dbus.c b/lib/roles/dbus/dbus.c index 1fd460b1f..456b869a8 100644 --- a/lib/roles/dbus/dbus.c +++ b/lib/roles/dbus/dbus.c @@ -42,13 +42,12 @@ /* * retreives existing or creates new shadow wsi for fd owned by dbus stuff. * - * Requires vhost lock + * Requires context + vhost lock */ static struct lws * __lws_shadow_wsi(struct lws_dbus_ctx *ctx, DBusWatch *w, int fd, int create_ok) { - size_t s = sizeof(struct lws); struct lws *wsi; if (fd < 0 || fd >= (int)ctx->vh->context->fd_limit_per_thread) { @@ -69,27 +68,18 @@ __lws_shadow_wsi(struct lws_dbus_ctx *ctx, DBusWatch *w, int fd, int create_ok) if (!create_ok) return NULL; -#if defined(LWS_WITH_EVENT_LIBS) - s += ctx->vh->context->event_loop_ops->evlib_size_wsi; -#endif - - wsi = lws_zalloc(s, "shadow wsi"); + /* requires context lock */ + wsi = __lws_wsi_create_with_role(ctx->vh->context, ctx->tsi, NULL); if (wsi == NULL) { lwsl_err("Out of mem\n"); return NULL; } -#if defined(LWS_WITH_EVENT_LIBS) - wsi->evlib_wsi = (uint8_t *)wsi + sizeof(*wsi); -#endif - lwsl_info("%s: creating shadow wsi\n", __func__); - wsi->a.context = ctx->vh->context; wsi->desc.sockfd = fd; lws_role_transition(wsi, 0, LRS_ESTABLISHED, &role_ops_dbus); wsi->a.protocol = ctx->vh->protocols; - wsi->tsi = ctx->tsi; wsi->shadow = 1; wsi->opaque_parent_data = ctx; ctx->w[0] = w; @@ -97,13 +87,12 @@ __lws_shadow_wsi(struct lws_dbus_ctx *ctx, DBusWatch *w, int fd, int create_ok) lws_vhost_bind_wsi(ctx->vh, wsi); if (__insert_wsi_socket_into_fds(ctx->vh->context, wsi)) { lwsl_err("inserting wsi socket into fds failed\n"); + ctx->vh->context->pt[(int)ctx->tsi].count_wsi_allocated--; lws_vhost_unbind_wsi(wsi); lws_free(wsi); return NULL; } - ctx->vh->context->pt[(int)ctx->tsi].count_wsi_allocated++; - return wsi; } @@ -157,11 +146,13 @@ lws_dbus_add_watch(DBusWatch *w, void *data) struct lws *wsi; int n; + lws_context_lock(pt->context, __func__); lws_pt_lock(pt, __func__); wsi = __lws_shadow_wsi(ctx, w, dbus_watch_get_unix_fd(w), 1); if (!wsi) { lws_pt_unlock(pt); + lws_context_unlock(pt->context); lwsl_err("%s: unable to get wsi\n", __func__); return FALSE; @@ -193,6 +184,7 @@ lws_dbus_add_watch(DBusWatch *w, void *data) __lws_change_pollfd(wsi, 0, lws_flags); lws_pt_unlock(pt); + lws_context_unlock(pt->context); return TRUE; } @@ -233,6 +225,7 @@ lws_dbus_remove_watch(DBusWatch *w, void *data) struct lws *wsi; int n; + lws_context_lock(pt->context, __func__); lws_pt_lock(pt, __func__); wsi = __lws_shadow_wsi(ctx, w, dbus_watch_get_unix_fd(w), 0); @@ -261,6 +254,7 @@ lws_dbus_remove_watch(DBusWatch *w, void *data) bail: lws_pt_unlock(pt); + lws_context_unlock(pt->context); } static void diff --git a/lib/roles/http/server/server.c b/lib/roles/http/server/server.c index 0d03f7dd8..2dbec559a 100644 --- a/lib/roles/http/server/server.c +++ b/lib/roles/http/server/server.c @@ -272,21 +272,16 @@ done_list: goto deal; } - { - size_t s = sizeof(struct lws); + /* + * Create the listen wsi and customize it + */ -#if defined(LWS_WITH_EVENT_LIBS) - s += vhost->context->event_loop_ops->evlib_size_wsi; -#endif - - wsi = lws_zalloc(s, "listen wsi"); - if (wsi == NULL) { - lwsl_err("Out of mem\n"); - goto bail; - } -#if defined(LWS_WITH_EVENT_LIBS) - wsi->evlib_wsi = (uint8_t *)wsi + sizeof(*wsi); -#endif + lws_context_lock(vhost->context, __func__); + wsi = __lws_wsi_create_with_role(vhost->context, m, &role_ops_listen); + lws_context_unlock(vhost->context); + if (wsi == NULL) { + lwsl_err("Out of mem\n"); + goto bail; } #ifdef LWS_WITH_UNIX_SOCK @@ -299,11 +294,8 @@ done_list: lwsl_debug("%s: lws_socket_bind says %d\n", __func__, is); } - wsi->a.context = vhost->context; wsi->desc.sockfd = sockfd; - lws_role_transition(wsi, 0, LRS_UNCONNECTED, &role_ops_listen); wsi->a.protocol = vhost->protocols; - wsi->tsi = m; lws_vhost_bind_wsi(vhost, wsi); wsi->listener = 1; @@ -319,7 +311,6 @@ done_list: goto bail; } - pt->count_wsi_allocated++; vhost->lserv_wsi = wsi; lws_pt_unlock(pt); diff --git a/lib/roles/netlink/ops-netlink.c b/lib/roles/netlink/ops-netlink.c index 9a0ecd72f..3bf19736b 100644 --- a/lib/roles/netlink/ops-netlink.c +++ b/lib/roles/netlink/ops-netlink.c @@ -352,8 +352,11 @@ rops_pt_init_destroy_netlink(struct lws_context *context, /* * We want a netlink socket per pt as well */ - wsi = lws_wsi_create_with_role(context, (int)(pt - &context->pt[0]), + + lws_context_lock(context, __func__); + wsi = __lws_wsi_create_with_role(context, (int)(pt - &context->pt[0]), &role_ops_netlink); + lws_context_unlock(context); if (!wsi) goto bail;