1
0
Fork 0
mirror of https://github.com/warmcat/libwebsockets.git synced 2025-03-09 00:00:04 +01:00

ss: formalize user cb retcodes

It's not safe to destroy objects inside a callback from a parent that
still has references to the object.

Formalize what the user code can indicate by its return code from the
callback functions and provide the implementations at the parents.

 - LWSSSSRET_OK:            no action, OK
 - LWSSSSRET_DISCONNECT_ME: disconnect the underlying connection
 - LWSSSSRET_DESTROY_ME:    destroy the ss object
 - LWSSSSRET_TX_DONT_SEND:  for tx, give up the tx opportunity since nothing to send
This commit is contained in:
Andy Green 2020-06-01 07:33:37 +01:00
parent e4ab18342a
commit 698eda63d7
10 changed files with 178 additions and 48 deletions

View file

@ -232,6 +232,18 @@ typedef enum {
} lws_ss_conn_states_t;
/*
* Returns from state() callback can tell the caller what the user code
* wants to do
*/
enum lws_ss_state_return_t {
LWSSSSRET_TX_DONT_SEND = 1, /* (*tx) only */
LWSSSSRET_OK = 0,
LWSSSSRET_DISCONNECT_ME = -1,
LWSSSSRET_DESTROY_ME = -2,
};
/**
* lws_ss_info_t: information about stream to be created
*

View file

@ -101,6 +101,16 @@ lws_ss_policy_metadata_index(const lws_ss_policy_t *p, size_t index)
return NULL;
}
static int
fe_lws_ss_destroy(struct lws_dll2 *d, void *user)
{
lws_ss_handle_t *h = lws_container_of(d, lws_ss_handle_t, list);
lws_ss_destroy(&h);
return 0;
}
int
lws_ss_policy_set(struct lws_context *context, const char *name)
{
@ -123,6 +133,19 @@ lws_ss_policy_set(struct lws_context *context, const char *name)
lejp_destruct(&args->jctx);
if (context->ac_policy) {
int n;
/*
* any existing ss created with the old policy have to go away
* now, since they point to the shortly-to-be-destroyed old
* policy
*/
for (n = 0; n < context->count_threads; n++) {
struct lws_context_per_thread *pt = &context->pt[n];
lws_dll2_foreach_safe(&pt->ss_owner, NULL, fe_lws_ss_destroy);
}
/*
* So this is a bit fun-filled, we already had a policy in

View file

@ -166,7 +166,7 @@ secstream_h1(struct lws *wsi, enum lws_callback_reasons reason, void *user,
lws_ss_handle_t *h = (lws_ss_handle_t *)lws_get_opaque_user_data(wsi);
uint8_t buf[LWS_PRE + 1520], *p = &buf[LWS_PRE],
*end = &buf[sizeof(buf) - 1];
int f = 0, m, status, txr;
int f = 0, m, status;
char conceal_eom = 0;
size_t buflen;
@ -177,7 +177,12 @@ secstream_h1(struct lws *wsi, enum lws_callback_reasons reason, void *user,
assert(h->policy);
lwsl_info("%s: h: %p, %s CLIENT_CONNECTION_ERROR: %s\n", __func__,
h, h->policy->streamtype, in ? (char *)in : "(null)");
lws_ss_event_helper(h, LWSSSCS_UNREACHABLE);
/* already disconnected, no action for DISCONNECT_ME */
if (lws_ss_event_helper(h, LWSSSCS_UNREACHABLE) ==
LWSSSSRET_DESTROY_ME) {
lws_ss_destroy(&h);
break;
}
h->wsi = NULL;
lws_ss_backoff(h);
break;
@ -201,7 +206,9 @@ secstream_h1(struct lws *wsi, enum lws_callback_reasons reason, void *user,
if (h->policy && !(h->policy->flags & LWSSSPOLF_OPPORTUNISTIC) &&
!h->txn_ok && !wsi->context->being_destroyed)
lws_ss_backoff(h);
if (lws_ss_event_helper(h, LWSSSCS_DISCONNECTED))
/* already disconnected, no action for DISCONNECT_ME */
if (lws_ss_event_helper(h, LWSSSCS_DISCONNECTED) ==
LWSSSSRET_DESTROY_ME)
lws_ss_destroy(&h);
break;
@ -443,13 +450,16 @@ malformed:
wsi->http.writeable_len = h->writeable_len = 0;
if (h->u.http.good_respcode)
lws_ss_event_helper(h, LWSSSCS_QOS_ACK_REMOTE);
else
lws_ss_event_helper(h, LWSSSCS_QOS_NACK_REMOTE);
if (h->u.http.good_respcode) {
if (lws_ss_event_helper(h, LWSSSCS_QOS_ACK_REMOTE))
break;
} else
if (lws_ss_event_helper(h, LWSSSCS_QOS_NACK_REMOTE))
break;
h->wsi = NULL;
h->txn_ok = 1;
//bad = status != 200;
lws_cancel_service(lws_get_context(wsi)); /* abort poll wait */
break;
@ -476,15 +486,23 @@ malformed:
#endif
txr = h->info.tx(ss_to_userobj(h), h->txord++, p, &buflen, &f);
if (txr < 0) {
lwsl_debug("%s: tx handler asked to close\n", __func__);
return -1;
}
if (txr > 0) {
switch(h->info.tx(ss_to_userobj(h), h->txord++, p, &buflen, &f)) {
case LWSSSSRET_DISCONNECT_ME:
lwsl_debug("%s: tx handler asked to close conn\n", __func__);
return -1; /* close connection */
case LWSSSSRET_DESTROY_ME:
lws_set_opaque_user_data(wsi, NULL);
h->wsi = NULL;
lws_ss_destroy(&h);
return -1; /* close connection */
case LWSSSSRET_TX_DONT_SEND:
/* don't want to send anything */
lwsl_debug("%s: dont want to write\n", __func__);
return 0;
default:
break;
}
lwsl_info("%s: WRITEABLE: user tx says len %d fl 0x%x\n",

View file

@ -31,8 +31,8 @@ secstream_mqtt(struct lws *wsi, enum lws_callback_reasons reason, void *user,
lws_ss_handle_t *h = (lws_ss_handle_t *)lws_get_opaque_user_data(wsi);
lws_mqtt_publish_param_t mqpp, *pmqpp;
uint8_t buf[LWS_PRE + 1400];
int f = 0, txr;
size_t buflen;
int f = 0;
switch (reason) {
@ -144,15 +144,25 @@ secstream_mqtt(struct lws *wsi, enum lws_callback_reasons reason, void *user,
buflen = sizeof(buf) - LWS_PRE;
txr = h->info.tx(ss_to_userobj(h), h->txord++, buf + LWS_PRE,
&buflen, &f);
if (txr < 0) {
lwsl_debug("%s: tx handler asked to close\n", __func__);
return -1;
}
if (txr > 0)
switch(h->info.tx(ss_to_userobj(h), h->txord++, buf + LWS_PRE,
&buflen, &f)) {
case LWSSSSRET_DISCONNECT_ME:
lwsl_debug("%s: tx handler asked to close conn\n", __func__);
return -1; /* close connection */
case LWSSSSRET_DESTROY_ME:
lws_set_opaque_user_data(wsi, NULL);
h->wsi = NULL;
lws_ss_destroy(&h);
return -1; /* close connection */
case LWSSSSRET_TX_DONT_SEND:
/* don't want to send anything */
lwsl_debug("%s: dont want to write\n", __func__);
return 0;
default:
break;
}
memset(&mqpp, 0, sizeof(mqpp));
mqpp.topic = (char *)h->policy->u.mqtt.topic;

View file

@ -33,8 +33,8 @@ secstream_raw(struct lws *wsi, enum lws_callback_reasons reason, void *user,
lws_ss_handle_t *h = (lws_ss_handle_t *)lws_get_opaque_user_data(wsi);
uint8_t buf[LWS_PRE + 1520], *p = &buf[LWS_PRE],
*end = &buf[sizeof(buf) - 1];
int f = 0, txr;
size_t buflen;
int f = 0;
switch (reason) {
@ -89,15 +89,23 @@ secstream_raw(struct lws *wsi, enum lws_callback_reasons reason, void *user,
return 0;
buflen = lws_ptr_diff(end, p);
txr = h->info.tx(ss_to_userobj(h), h->txord++, p, &buflen, &f);
if (txr < 0) {
lwsl_debug("%s: tx handler asked to close\n", __func__);
return -1;
}
if (txr > 0) {
switch(h->info.tx(ss_to_userobj(h), h->txord++, p, &buflen, &f)) {
case LWSSSSRET_DISCONNECT_ME:
lwsl_debug("%s: tx handler asked to close conn\n", __func__);
return -1; /* close connection */
case LWSSSSRET_DESTROY_ME:
lws_set_opaque_user_data(wsi, NULL);
h->wsi = NULL;
lws_ss_destroy(&h);
return -1; /* close connection */
case LWSSSSRET_TX_DONT_SEND:
/* don't want to send anything */
lwsl_debug("%s: dont want to write\n", __func__);
return 0;
default:
break;
}
/*

View file

@ -30,7 +30,7 @@ secstream_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
{
lws_ss_handle_t *h = (lws_ss_handle_t *)lws_get_opaque_user_data(wsi);
uint8_t buf[LWS_PRE + 1400];
int f = 0, f1, txr;
int f = 0, f1;
size_t buflen;
switch (reason) {
@ -41,7 +41,10 @@ secstream_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
in ? (char *)in : "(null)");
if (!h)
break;
lws_ss_event_helper(h, LWSSSCS_UNREACHABLE);
if (lws_ss_event_helper(h, LWSSSCS_UNREACHABLE)) {
lws_ss_destroy(&h);
break;
}
h->wsi = NULL;
lws_ss_backoff(h);
break;
@ -100,15 +103,25 @@ secstream_ws(struct lws *wsi, enum lws_callback_reasons reason, void *user,
}
buflen = sizeof(buf) - LWS_PRE;
txr = h->info.tx(ss_to_userobj(h), h->txord++, buf + LWS_PRE,
&buflen, &f);
if (txr < 0) {
lwsl_debug("%s: tx handler asked to close\n", __func__);
return -1;
}
if (txr > 0)
switch(h->info.tx(ss_to_userobj(h), h->txord++, buf + LWS_PRE,
&buflen, &f)) {
case LWSSSSRET_DISCONNECT_ME:
lwsl_debug("%s: tx handler asked to close conn\n", __func__);
return -1; /* close connection */
case LWSSSSRET_DESTROY_ME:
lws_set_opaque_user_data(wsi, NULL);
h->wsi = NULL;
lws_ss_destroy(&h);
return -1; /* close connection */
case LWSSSSRET_TX_DONT_SEND:
/* don't want to send anything */
lwsl_debug("%s: dont want to write\n", __func__);
return 0;
default:
break;
}
f1 = lws_write_ws_flags(LWS_WRITE_BINARY,
!!(f & LWSSS_FLAG_SOM),

View file

@ -77,6 +77,8 @@ lws_ss_state_name(int state)
int
lws_ss_event_helper(lws_ss_handle_t *h, lws_ss_constate_t cs)
{
int n;
if (!h)
return 0;
@ -90,12 +92,38 @@ lws_ss_event_helper(lws_ss_handle_t *h, lws_ss_constate_t cs)
(void *)h, NULL);
#endif
if (h->h_sink && h->h_sink->info.state &&
h->h_sink->info.state(h->sink_obj, h->h_sink, cs, 0))
return 1;
if (h->h_sink && h->h_sink->info.state) {
n = h->h_sink->info.state(h->sink_obj, h->h_sink, cs, 0);
if (n) {
lws_set_timeout(h->wsi, 1, LWS_TO_KILL_ASYNC);
h->wsi = NULL; /* stop destroy trying to repeat this */
if (n == LWSSSSRET_DESTROY_ME) {
lws_ss_destroy(&h);
return 1;
}
}
}
if (h->info.state)
return h->info.state(ss_to_userobj(h), NULL, cs, 0);
if (h->info.state) {
n = h->info.state(ss_to_userobj(h), NULL, cs, 0);
if (n) {
if (cs == LWSSSCS_CREATING)
/* just let caller handle it */
return 1;
if (h->wsi)
lws_set_timeout(h->wsi, 1, LWS_TO_KILL_ASYNC);
if (n == LWSSSSRET_DESTROY_ME) {
lwsl_info("%s: ss %p asks to be destroyed\n", __func__, h);
/* disconnect ss from the wsi */
if (h->wsi)
lws_set_opaque_user_data(h->wsi, NULL);
h->wsi = NULL; /* stop destroy trying to repeat this */
lws_ss_destroy(&h);
return 1;
}
h->wsi = NULL; /* stop destroy trying to repeat this */
}
}
return 0;
}
@ -105,7 +133,7 @@ lws_ss_timeout_sul_check_cb(lws_sorted_usec_list_t *sul)
{
lws_ss_handle_t *h = lws_container_of(sul, lws_ss_handle_t, sul);
lwsl_err("%s: retrying ss h %p after backoff\n", __func__, h);
lwsl_err("%s: retrying ss h %p (%s) after backoff\n", __func__, h, h->policy->streamtype);
/* we want to retry... */
h->seqstate = SSSEQ_DO_RETRY;
@ -481,7 +509,14 @@ lws_ss_create(struct lws_context *context, int tsi, const lws_ss_info_t *ssi,
*/
}
lws_ss_event_helper(h, LWSSSCS_CREATING);
if (lws_ss_event_helper(h, LWSSSCS_CREATING)) {
lws_pt_lock(pt, __func__);
lws_dll2_remove(&h->list);
lws_pt_unlock(pt);
lws_free(h);
return 1;
}
if (!ssi->register_sink && (h->policy->flags & LWSSSPOLF_NAILED_UP))
if (lws_ss_client_connect(h))
@ -511,7 +546,7 @@ lws_ss_destroy(lws_ss_handle_t **ppss)
* Don't let the wsi point to us any more,
* we (the ss object bound to the wsi) are going away now
*/
// lws_set_opaque_user_data(h->wsi, NULL);
lws_set_opaque_user_data(h->wsi, NULL);
lws_set_timeout(h->wsi, 1, LWS_TO_KILL_SYNC);
}
@ -521,6 +556,7 @@ lws_ss_destroy(lws_ss_handle_t **ppss)
*ppss = NULL;
lws_dll2_remove(&h->list);
lws_dll2_remove(&h->to_list);
/* no need to worry about return code since we are anyway destroying */
lws_ss_event_helper(h, LWSSSCS_DESTROYING);
lws_pt_unlock(pt);

View file

@ -52,7 +52,7 @@ ss_cpd_state(void *userobj, void *sh, lws_ss_constate_t state,
break;
case LWSSSCS_QOS_ACK_REMOTE:
lws_system_cpd_set(cx, LWS_CPD_INTERNET_OK);
break;
return LWSSSSRET_DESTROY_ME;
case LWSSSCS_ALL_RETRIES_FAILED:
case LWSSSCS_DISCONNECTED:
@ -61,13 +61,13 @@ ss_cpd_state(void *userobj, void *sh, lws_ss_constate_t state,
* cover the situation we didn't connect to anything
*/
lws_system_cpd_set(cx, LWS_CPD_NO_INTERNET);
break;
return LWSSSSRET_DESTROY_ME;
default:
break;
}
return 0;
return LWSSSSRET_OK;
}
int

View file

@ -77,6 +77,8 @@ policy_set(lws_sorted_usec_list_t *sul)
* ss connection close that was using the vhost from the old policy
*/
lws_ss_destroy(&m->ss);
if (lws_ss_policy_set(context, "updated"))
lwsl_err("%s: policy set failed\n", __func__);
else {

View file

@ -16,6 +16,14 @@ require_lws_config(LWS_WITH_SECURE_STREAMS_STATIC_POLICY_ONLY 0 requirements)
if (requirements)
add_executable(${SAMP} minimal-secure-streams.c)
if (LWS_CTEST_INTERNET_AVAILABLE)
add_test(NAME ss-warmcat COMMAND lws-minimal-secure-streams)
set_tests_properties(ss-warmcat
PROPERTIES
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/minimal-examples/secure-streams/minimal-secure-streams
TIMEOUT 20)
endif()
if (websockets_shared)
target_link_libraries(${SAMP} websockets_shared)
add_dependencies(${SAMP} websockets_shared)