From: Seonah Moon Date: Mon, 7 Jun 2021 01:09:58 +0000 (+0900) Subject: Select proper vhost when connecting with a client X-Git-Tag: accepted/tizen/unified/20210622.125832~6 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fchanges%2F65%2F259365%2F6;p=platform%2Fcore%2Fapi%2Fvine.git Select proper vhost when connecting with a client [problem] Sometimes, Segfault occurs when pubsub open/close repeatedly for the same port. [cause] libwebsockets doesn't close the vhost and a listen socket immediately eventhough a vine requests it. After then, if the accept event occurs, libwebsockets returns old vhost which has the same listen port from the vhost_list. Change-Id: Ieded5f672b077939a126764b99ef1f052a01461b --- diff --git a/packaging/capi-network-vine.spec b/packaging/capi-network-vine.spec index 8c245c4..473e4ac 100755 --- a/packaging/capi-network-vine.spec +++ b/packaging/capi-network-vine.spec @@ -2,7 +2,7 @@ %bcond_without lws_static_prebuilt Name: capi-network-vine Summary: An service discovery framework -Version: 1.0.3 +Version: 1.0.5 Release: 0 Group: Network & Connectivity/API License: Apache-2.0 @@ -96,6 +96,7 @@ export LDFLAGS+=" -lgcov" -DUSE_LIBWEBSOCKETS_STATIC_PREBUILT=OFF \ %endif -DWITH_UNITTEST=ON \ + -DENABLE_DATAPATH_PLUGIN_DEBUG=ON \ -DWITH_VINE_TEST=ON make %{?jobs:-j%jobs} diff --git a/plugins/libwebsockets/libwebsockets-plugin.cpp b/plugins/libwebsockets/libwebsockets-plugin.cpp index a16c942..ed0c6fe 100755 --- a/plugins/libwebsockets/libwebsockets-plugin.cpp +++ b/plugins/libwebsockets/libwebsockets-plugin.cpp @@ -16,7 +16,7 @@ */ #include - +#include #include #include #include @@ -32,6 +32,7 @@ #include "vine-log.h" #include "vine-utils.h" #include "vine-queue.h" +#include "vine-set.h" #ifdef ENABLE_DATAPATH_PLUGIN_DEBUG #define DEBUG_LEVEL (LLL_USER | LLL_ERR | LLL_WARN | LLL_DEBUG | LLL_NOTICE | LLL_INFO) @@ -51,10 +52,12 @@ typedef struct { typedef struct { struct lws *wsi; struct lws_vhost *vh; + bool is_server; bool close_requested; int curr_conn; int max_conn; + char *host_name; unsigned char *token; int token_len; @@ -82,6 +85,7 @@ typedef struct { } websocket_op_s; static VineQueue op_queue; +static VineSet listen_vh_list; static int g_ref_count = 0; static pthread_mutex_t g_lws_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -375,6 +379,18 @@ static void _invoke_connected_cb(struct lws *wsi, bool connected, void *user_dat g_callbacks.connected_cb(0, user_data); } +static int __check_vhost(struct lws_vhost *vh) +{ + auto it = listen_vh_list.find(vh); + if (it == listen_vh_list.end()) { + VINE_LOGE("vh[%p] isn't used. port[%d]", + vh, lws_get_vhost_port(vh)); + return 0; + } + + return 1; +} + static int _websocket_protocol_cb(struct lws *wsi, enum lws_callback_reasons reason, void *user, void *in, size_t len) { @@ -466,12 +482,19 @@ static int _websocket_protocol_cb(struct lws *wsi, case LWS_CALLBACK_ESTABLISHED: { VINE_LOGI("Websocket connection is established."); websocket_s *client_ws = (websocket_s *)lws_wsi_user(wsi); + if (!__check_vhost(lws_get_vhost(wsi))) { + VINE_LOGE("Ignore a connection."); + return -1; + } vhd->curr_conn++; if (g_callbacks.accepted_cb) { vine_dp_addr_family_e addr_family; char ip[INET6_ADDRSTRLEN]; int port; _get_peer_network_info(wsi, &addr_family, ip, &port); + VINE_LOGE("vh[%p] wsi[%p] vhd->user[%p] port[%d]", + lws_get_vhost(wsi), + wsi, vhd->user, lws_get_vhost_port(lws_get_vhost(wsi))); g_callbacks.accepted_cb(addr_family, ip, port, client_ws, vhd->user); } break; @@ -498,7 +521,8 @@ static int _websocket_protocol_cb(struct lws *wsi, } if (ws->close_requested) { - VINE_LOGI("Close websocket."); + VINE_LOGI("Close websocket. ws[%p] ws->user[%p] wsi[%p]", + ws, ws->user, wsi); return -1; } @@ -610,6 +634,13 @@ static int websocket_init(void) return VINE_DATA_PATH_ERROR_NONE; } +static void _clear_listen_vhosts(void) +{ + for (auto &vh : listen_vh_list) + lws_vhost_destroy(vh); + listen_vh_list.clear(); +} + static void websocket_deinit(void) { if (__sync_sub_and_fetch(&g_ref_count, 1) > 0) { @@ -626,6 +657,7 @@ static void websocket_deinit(void) it = g_pollfds.erase(it); } + _clear_listen_vhosts(); VINE_LOGD("lws is deinitialized."); } @@ -655,7 +687,13 @@ static long __get_ssl_ctx_options(vine_dp_tls_version_e tls_version) return options; } -static struct lws_vhost *_create_vhost(int addr_family, +static void __vhost_finalized_cb(struct lws_vhost *vh, void *arg) +{ + VINE_LOGE("vh[%p] is finalized", vh); + listen_vh_list.erase(vh); +} + +static struct lws_vhost *_create_vhost(const char *host_name, int addr_family, int port, const char *iface_name, vine_dp_ssl ssl) { struct lws_context_creation_info info; @@ -676,9 +714,9 @@ static struct lws_vhost *_create_vhost(int addr_family, info.ssl_cert_filepath = ssl.cert_path; info.ssl_private_key_filepath = ssl.key_path; info.protocols = protocols; - info.vhost_name = "vine-websocket-server"; + info.vhost_name = host_name ? STRDUP(host_name) : "vine-websocket-server"; info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT | LWS_SERVER_OPTION_EXPLICIT_VHOSTS; - + info.finalize = __vhost_finalized_cb; if (addr_family == VINE_DP_IPV4) info.options |= LWS_SERVER_OPTION_DISABLE_IPV6; else if (addr_family == VINE_DP_IPV6) @@ -692,7 +730,7 @@ static void _open_server(websocket_s *ws, int addr_family, int port, const char *iface_name, int max_conn, vine_dp_ssl ssl) { ws->max_conn = max_conn; - ws->vh = _create_vhost(addr_family, port, iface_name, ssl); + ws->vh = _create_vhost(ws->host_name, addr_family, port, iface_name, ssl); if (!ws->vh) { VINE_LOGE("Failed to create vhost."); if (g_callbacks.opened_cb) @@ -700,12 +738,16 @@ static void _open_server(websocket_s *ws, int addr_family, return; } + //listen_vh_list.insert((void *)ws->vh); + listen_vh_list.insert(ws->vh); + void *user_data = lws_protocol_vh_priv_zalloc(ws->vh, protocols, sizeof(websocket_s)); memcpy(user_data, ws, sizeof(websocket_s)); int vport = lws_get_vhost_port(ws->vh); if (g_callbacks.opened_cb) g_callbacks.opened_cb(VINE_DATA_PATH_ERROR_NONE, vport, ws->user); + VINE_LOGD("vh[%p] user[%p] port[%d]", ws->vh, ws->user, vport); } static int websocket_open(vine_dp_plugin_h handle, @@ -714,6 +756,9 @@ static int websocket_open(vine_dp_plugin_h handle, RET_VAL_IF(!handle, VINE_DATA_PATH_ERROR_INVALID_PARAMETER, "handle is NULL"); RET_VAL_IF(!g_context, VINE_DATA_PATH_ERROR_INVALID_OPERATION, "g_context is NULL"); + websocket_s *ws = (websocket_s *)handle; + ws->is_server = true; + VINE_LOGD("ws[%p]", ws); if (_add_websocket_op_request(WEBSOCKET_OP_OPEN, (websocket_s *)handle, addr_family, NULL, port, iface_name, max_conn, &ssl) < 0) return VINE_DATA_PATH_ERROR_OPERATION_FAILED; @@ -734,7 +779,7 @@ static void _connect_server(websocket_s *ws, int addr_family, const char *ip, in client_conn_info.address = ip; client_conn_info.iface = iface_name; client_conn_info.path = "/"; - client_conn_info.host = client_conn_info.address; + client_conn_info.host = ws->host_name ? ws->host_name : client_conn_info.address; client_conn_info.origin = client_conn_info.address; client_conn_info.protocol = "vine-websocket-protocol"; @@ -748,7 +793,8 @@ static void _connect_server(websocket_s *ws, int addr_family, const char *ip, in client_conn_info.ssl_connection |= LCCSCF_SKIP_SERVER_CERT_HOSTNAME_CHECK; client_conn_info.userdata = (void *)ws; - client_conn_info.vhost = _create_vhost(addr_family, CONTEXT_PORT_NO_LISTEN, iface_name, ssl); + client_conn_info.vhost = _create_vhost(ws->host_name, + addr_family, CONTEXT_PORT_NO_LISTEN, iface_name, ssl); ws->wsi = lws_client_connect_via_info(&client_conn_info); } @@ -930,6 +976,14 @@ static int websocket_close(vine_dp_plugin_h handle) websocket_s *ws = (websocket_s *)handle; ws->close_requested = true; + if (ws->is_server) { + VINE_LOGE("vh destroy[%p]", ws->vh); + listen_vh_list.erase(ws->vh); + lws_vhost_destroy(ws->vh); + ws->vh = NULL; + return VINE_DATA_PATH_ERROR_NONE; + } + if (ws->wsi) lws_callback_on_writable(ws->wsi); @@ -973,7 +1027,7 @@ static int websocket_create(vine_dp_plugin_h *handle, void *plugin_data, void *u (websocket_s *)plugin_data : (websocket_s *)calloc(1, sizeof(websocket_s)); RET_VAL_IF(ws == NULL, VINE_DATA_PATH_ERROR_OUT_OF_MEMORY, "Out of memory"); - VINE_LOGD("plugin_data[%p]", plugin_data); + VINE_LOGD("ws[%p], user[%p]", ws, user); ws->close_requested = false; ws->curr_conn = 0; @@ -990,12 +1044,15 @@ static int websocket_destroy(vine_dp_plugin_h handle) { RET_VAL_IF(handle == NULL, VINE_DATA_PATH_ERROR_INVALID_PARAMETER, "handle is NULL"); + VINE_LOGE("ws[%p] is destroyed", handle); websocket_s *ws = (websocket_s *)handle; ws->wsi = NULL; ws->vh = NULL; ws->user = NULL; + free(ws->host_name); + ws->host_name = NULL; free(ws->token); ws->token = NULL; ws->token_len = 0; @@ -1059,6 +1116,18 @@ static int websocket_get_token(vine_dp_plugin_h handle, char **token) return VINE_DATA_PATH_ERROR_NONE; } +static int websocket_set_host_name(vine_dp_plugin_h handle, const char *name) +{ + RET_VAL_IF(handle == NULL, VINE_DATA_PATH_ERROR_INVALID_PARAMETER, "handle is NULL"); + RET_VAL_IF(name == NULL, VINE_DATA_PATH_ERROR_INVALID_PARAMETER, "name is NULL"); + + websocket_s *ws = (websocket_s *)handle; + if (ws->host_name) + free(ws->host_name); + ws->host_name = strdup(name); + return VINE_DATA_PATH_ERROR_NONE; +} + void vine_data_path_plugin_init(vine_dp_plugin_fn *fn) { fn->init = websocket_init; @@ -1078,4 +1147,5 @@ void vine_data_path_plugin_init(vine_dp_plugin_fn *fn) fn->set_token = websocket_set_token; fn->get_token = websocket_get_token; + fn->set_host_name = websocket_set_host_name; } diff --git a/src/include/vine-data-path-plugin.h b/src/include/vine-data-path-plugin.h index 3f8a4c9..214d0d8 100755 --- a/src/include/vine-data-path-plugin.h +++ b/src/include/vine-data-path-plugin.h @@ -110,6 +110,8 @@ typedef struct { // DPPubSub uses it to inform own service name to a peer. int (*set_token)(vine_dp_plugin_h handle, const char *token); int (*get_token)(vine_dp_plugin_h handle, char **token); + + int (*set_host_name)(vine_dp_plugin_h handle, const char *name); } vine_dp_plugin_fn; extern "C" void vine_data_path_plugin_init(vine_dp_plugin_fn *fn); diff --git a/src/include/vine-data-path.h b/src/include/vine-data-path.h index 15421ff..f25ed47 100755 --- a/src/include/vine-data-path.h +++ b/src/include/vine-data-path.h @@ -36,14 +36,15 @@ int vine_data_path_init(void); int vine_data_path_deinit(void); int vine_data_path_open(vine_address_family_e addr_family, int port, - const char *iface_name, int max_conn, vine_security_h security, + const char *iface_name, int max_conn, + vine_security_h security, const char *host_name, vine_data_path_opened_cb opened_cb, void *opened_cb_data, vine_data_path_accepted_cb accepted_cb, void *accepted_cb_data, vine_data_path_h *opened_datapath, vine_event_queue_h event_fd); int vine_data_path_connect(vine_address_family_e addr_family, const char *ip, int port, const char *iface_name, - vine_security_h security, const char *token, + vine_security_h security, const char *host_name, const char *token, vine_data_path_connected_cb callback, void *user_data, vine_data_path_h *connected_datapath, vine_event_queue_h event_fd); diff --git a/src/include/vine-set.h b/src/include/vine-set.h new file mode 100755 index 0000000..3be5dcd --- /dev/null +++ b/src/include/vine-set.h @@ -0,0 +1,92 @@ +/* + * Copyright (c) 2021 Samsung Electronics Co., Ltd. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * +*/ +#pragma once + +#include +#include +#include + +#include "vine-log.h" + +template +class VineSet +{ +public: + using iterator = typename std::set::iterator; + VineSet() + { + VINE_LOGD("New Set"); + } + virtual ~VineSet() + { + VINE_LOGD("Destroy Set"); + } + + void insert(const T &element) + { + std::lock_guard lock_guard(_s_mutex); + _set.insert(element); + } + + void erase(const T &element) + { + std::lock_guard lock_guard(_s_mutex); + if (_set.empty()) + return; + _set.erase(element); + } + + void clear() + { + std::lock_guard lock_guard(_s_mutex); + _set.clear(); + } + + size_t size() + { + std::lock_guard lock_guard(_s_mutex); + return _set.size(); + } + + bool empty() + { + std::lock_guard lock_guard(_s_mutex); + return _set.empty(); + } + + iterator find(const T &element) + { + std::lock_guard lock_guard(_s_mutex); + return _set.find(element); + } + + iterator begin() + { + std::lock_guard lock_guard(_s_mutex); + return _set.begin(); + } + + iterator end() + { + std::lock_guard lock_guard(_s_mutex); + return _set.end(); + } + +private: + std::set _set; + std::mutex _s_mutex; +}; diff --git a/src/vine-data-path.cpp b/src/vine-data-path.cpp index d20890f..f217c61 100755 --- a/src/vine-data-path.cpp +++ b/src/vine-data-path.cpp @@ -271,6 +271,8 @@ static void __accepted_cb(vine_dp_addr_family_e addr_family, char *addr, RET_IF(user_data == NULL, "listen_dp is NULL"); vine_data_path_s *listen_dp = (vine_data_path_s *)user_data; + + VINE_LOGE("listen_dp[%p], security[%p]", listen_dp, listen_dp->security); vine_data_path_s *connected_dp = _vine_data_path_create(__convert_addr_family(addr_family), addr, port, listen_dp->security, plugin_data, listen_dp->event_fd); RET_IF(connected_dp == NULL, "Out of memory"); @@ -507,14 +509,13 @@ static vine_data_path_s *_vine_data_path_create(vine_address_family_e addr_famil if (ret != VINE_DATA_PATH_ERROR_NONE) { free(dp->addr); _vine_security_destroy(dp->security); + dp->security = NULL; free(dp); return NULL; } dp->state = vine_get_default_state(dp, dp->plugin_handle); - VINE_LOGD("datapath[%p] is created.", dp); - return dp; } @@ -527,9 +528,13 @@ int vine_data_path_destroy(vine_data_path_h datapath) dp->plugin_handle = NULL; free(dp->addr); dp->addr = NULL; + _vine_security_destroy(dp->security); + dp->security = NULL; + dp->listen_dp = NULL; delete dp->state; - free(datapath); + VINE_LOGD("data_path[%p] is destroyed", datapath); + free(datapath); return VINE_ERROR_NONE; } @@ -667,7 +672,7 @@ static void _destroy_security_info(vine_dp_ssl *dest) } int vine_data_path_open(vine_address_family_e addr_family, int port, const char *iface_name, - int max_conn, vine_security_h security, + int max_conn, vine_security_h security, const char *host_name, vine_data_path_opened_cb opened_cb, void *opened_cb_data, vine_data_path_accepted_cb accepted_cb, void *accepted_cb_data, vine_data_path_h *opened_datapath, @@ -692,6 +697,15 @@ int vine_data_path_open(vine_address_family_e addr_family, int port, const char dp->accepted_cb = accepted_cb; dp->accepted_cb_data = accepted_cb_data; + // optional + if (host_name) { + ret = g_dp_plugin_fn.set_host_name(dp->plugin_handle, host_name); + if (ret != VINE_DATA_PATH_ERROR_NONE) { + vine_data_path_destroy(dp); + return __convert_data_path_error_to_vine_error((vine_data_path_error)ret); + } + } + ret = g_dp_plugin_fn.open(dp->plugin_handle, dp_addr_family, port, iface_name, max_conn, ssl); _destroy_security_info(&ssl); @@ -711,7 +725,7 @@ int vine_data_path_open(vine_address_family_e addr_family, int port, const char int vine_data_path_connect(vine_address_family_e addr_family, const char *ip, int port, const char *iface_name, - vine_security_h security, const char *token, + vine_security_h security, const char *host_name, const char *token, vine_data_path_connected_cb callback, void *user_data, vine_data_path_h *connected_datapath, vine_event_queue_h event_fd) { @@ -728,6 +742,16 @@ int vine_data_path_connect(vine_address_family_e addr_family, _extract_security_info(security, &ssl); int ret; + + // optional + if (host_name) { + ret = g_dp_plugin_fn.set_host_name(dp->plugin_handle, host_name); + if (ret != VINE_DATA_PATH_ERROR_NONE) { + vine_data_path_destroy(dp); + return __convert_data_path_error_to_vine_error((vine_data_path_error)ret); + } + } + if (token) { ret = g_dp_plugin_fn.set_token(dp->plugin_handle, token); if (ret != VINE_DATA_PATH_ERROR_NONE) { diff --git a/src/vine-dp.cpp b/src/vine-dp.cpp index e0af7fa..1349b7c 100644 --- a/src/vine-dp.cpp +++ b/src/vine-dp.cpp @@ -601,7 +601,7 @@ int DPServer::open(vine_dp_opened_cb callback, void *user_data) int ret = vine_data_path_open(mAddrFamily, mListenPort, mIfaceName.size() > 0 ? mIfaceName.c_str() : NULL, - mMaxConnNum, mSecurity, + mMaxConnNum, mSecurity, NULL, _opened_cb, static_cast(this), _accepted_cb, static_cast(this), &mDataPath, mEventFd); @@ -791,7 +791,7 @@ int DPClient::open(vine_dp_opened_cb callback, void *user_data) int ret = vine_data_path_connect(mAddrFamily, mPeerIp.c_str(), mPeerPort, mIfaceName.size() > 0 ? mIfaceName.c_str() : NULL, - mSecurity, NULL, + mSecurity, NULL, NULL, _connected_cb, static_cast(this), &mDataPath, mEventFd); if (ret != VINE_ERROR_NONE) mOpenState = VINE_DP_OPEN_STATE_NONE; @@ -1004,7 +1004,7 @@ int DPPubSub::connect(const char *service_name, const char *ip, int port) conn_data->service_name = STRDUP(service_name); int ret = vine_data_path_connect(mAddrFamily, ip, port, iface_name, - mSecurity, mId.c_str(), + mSecurity, service_name, mId.c_str(), _pubsub_connected_cb, (void *)conn_data, &datapath, mEventFd); return ret; @@ -1023,6 +1023,7 @@ void DPPubSub::create_id(char id[]) const size_t map_len = strlen(map); string rand_str; + srand(time(NULL)); generate_n(back_inserter(rand_str), 6, [&](){ return map[rand() % map_len]; @@ -1036,7 +1037,6 @@ int DPPubSub::publish_service() vine_service_h service; int ret; char rank_str[VINE_DP_PUBSUB_RANK_LEN] = {0 , }; - char service_name[VINE_MAX_SERVICE_NAME_LEN + 1] = {0 , }; ret = vine_service_create(&service); if (ret != VINE_ERROR_NONE) @@ -1048,10 +1048,7 @@ int DPPubSub::publish_service() mRank = create_rank(); sprintf(rank_str, "%d", mRank); - create_id(service_name); - set_id(service_name); - - vine_service_set_name(service, service_name); + vine_service_set_name(service, mId.c_str()); vine_service_add_attribute(service, VINE_DP_PUBSUB_RANK_KEY, (const char *)rank_str); if (mSdPub == NULL) { @@ -1112,9 +1109,13 @@ int DPPubSub::open(vine_dp_opened_cb callback, void *user_data) mOpenedCb = callback; mOpenedCbData = user_data; + char service_name[VINE_MAX_SERVICE_NAME_LEN + 1] = {0 , }; + create_id(service_name); + set_id(service_name); + int ret = vine_data_path_open(mAddrFamily, mListenPort, mIfaceName.size() > 0 ? mIfaceName.c_str() : NULL, - mMaxConnNum, mSecurity, + mMaxConnNum, mSecurity, mId.c_str(), _pubsub_opened_cb, static_cast(this), _pubsub_accepted_cb, static_cast(this), &mServerDataPath, mEventFd); diff --git a/tests/vine-test/vine-pubsub-open-close-test.cpp b/tests/vine-test/vine-pubsub-open-close-test.cpp index fcc2ab0..17b6336 100644 --- a/tests/vine-test/vine-pubsub-open-close-test.cpp +++ b/tests/vine-test/vine-pubsub-open-close-test.cpp @@ -18,7 +18,7 @@ * Test: vine-pubsub-open-close-test * Description: * a PubSub DP opens and closes repeatedly in one session. - * The interval is 100ms. (DP_LIFETIME) + * The interval is 500ms. (DP_LIFETIME) */ #include @@ -34,7 +34,7 @@ #define MAX_EVENTS 50 #define DEFAULT_REPETITION_TIME 100 -#define DP_LIFETIME 100000000 // nanoseconds +#define DP_LIFETIME 500000000 // nanoseconds #define TOPIC "pubsub-test" static int epollfd = 0; @@ -206,8 +206,10 @@ static void open_dp(vine_session_h session) PRINT_IF_ERROR(vine_dp_open(dp, [](vine_dp_h dp, vine_error_e result, void *user_data) { if (result != VINE_ERROR_NONE) - return; - PRINT_LOG("dp[%p] is opened.", dp); + return; + int port = 0; + vine_dp_get_port(dp, &port); + PRINT_LOG("dp[%p] is opened. port[%d]", dp, port); _start_timer(dp, DP_LIFETIME); }, NULL), "vine_dp_open"); }