Skip to content
Permalink
Browse files

samples: net: echo_client: Fix bug in workqueue processing

The `start_udp_and_tcp` funcion is a blocking function, therefore it
should not be called from the system workqueue, as it would stall it.
Because it was called in such a way, the retry mechanism, which
also relied on the system workqueue did not work properly.

This commit fixes the issue, by keeping the main application processing
in the main thread, and using a semaphore to synchronize with the
connection manager.

Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
  • Loading branch information...
rlubos authored and jukkar committed Jul 25, 2019
1 parent ddf2230 commit 54eb4adff95879144a2e7d2664c7fdfbb77ee70e
Showing with 49 additions and 31 deletions.
  1. +49 −31 samples/net/sockets/echo_client/src/echo-client.c
@@ -81,8 +81,11 @@ struct configs conf = {
},
};

struct pollfd fds[4];
int nfds;
static struct pollfd fds[4];
static int nfds;

static bool connected;
K_SEM_DEFINE(run_app, 0, 1);

static struct net_mgmt_event_callback mgmt_cb;

@@ -123,7 +126,7 @@ static void wait(void)
}
}

static void start_udp_and_tcp(struct k_work *work)
static int start_udp_and_tcp(void)
{
int ret;

@@ -132,39 +135,46 @@ static void start_udp_and_tcp(struct k_work *work)
if (IS_ENABLED(CONFIG_NET_TCP)) {
ret = start_tcp();
if (ret < 0) {
return;
return ret;
}
}

if (IS_ENABLED(CONFIG_NET_UDP)) {
ret = start_udp();
if (ret < 0) {
return;
return ret;
}
}

prepare_fds();

while (true) {
if (IS_ENABLED(CONFIG_NET_TCP)) {
ret = process_tcp();
if (ret < 0) {
break;
}
}
return 0;
}

static int run_udp_and_tcp(void)
{
int ret;

wait();

if (IS_ENABLED(CONFIG_NET_UDP)) {
ret = process_udp();
if (ret < 0) {
break;
}
if (IS_ENABLED(CONFIG_NET_TCP)) {
ret = process_tcp();
if (ret < 0) {
return ret;
}
}

wait();
if (IS_ENABLED(CONFIG_NET_UDP)) {
ret = process_udp();
if (ret < 0) {
return ret;
}
}

return 0;
}

static void stop_udp_and_tcp(struct k_work *work)
static void stop_udp_and_tcp(void)
{
LOG_INF("Stopping...");

@@ -177,15 +187,6 @@ static void stop_udp_and_tcp(struct k_work *work)
}
}

static void do_service(k_work_handler_t handler)
{
static struct k_work work;

k_work_init(&work, handler);

k_work_submit(&work);
}

static void event_handler(struct net_mgmt_event_callback *cb,
u32_t mgmt_event, struct net_if *iface)
{
@@ -196,15 +197,17 @@ static void event_handler(struct net_mgmt_event_callback *cb,
if (mgmt_event == NET_EVENT_L4_CONNECTED) {
LOG_INF("Network connected");

do_service(start_udp_and_tcp);
connected = true;
k_sem_give(&run_app);

return;
}

if (mgmt_event == NET_EVENT_L4_DISCONNECTED) {
LOG_INF("Network disconnected");

do_service(stop_udp_and_tcp);
connected = false;
k_sem_reset(&run_app);

return;
}
@@ -237,13 +240,28 @@ static void init_app(void)

void main(void)
{
int ret;

init_app();

if (!IS_ENABLED(CONFIG_NET_CONNECTION_MANAGER)) {
/* If the config library has not been configured to start the
* app only after we have a connection, then we can start
* it right away.
*/
do_service(start_udp_and_tcp);
k_sem_give(&run_app);
}

while (true) {
/* Wait for the connection. */
k_sem_take(&run_app, K_FOREVER);

ret = start_udp_and_tcp();

while (connected && (ret == 0)) {
ret = run_udp_and_tcp();
}

stop_udp_and_tcp();
}
}

0 comments on commit 54eb4ad

Please sign in to comment.
You can’t perform that action at this time.