From 6c50e707d870f54401e441998980998367dd0621 Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Thu, 30 Nov 2023 13:02:25 +0100 Subject: [PATCH 1/6] serial: Use non-blocking reads in a polling loop Instead of doing blocking reads, which are not cancellable and therefore cause deadlocks when the .shutdown callback is called, use a fast polling loop with non-blocking reads. This fixes the read thread not closing on shutdown. Signed-off-by: Paul Cercueil --- serial.c | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/serial.c b/serial.c index 3ce96f208..4f080b6fa 100644 --- a/serial.c +++ b/serial.c @@ -13,18 +13,25 @@ #include #include +#include #include #include #include #include #include -#include +#ifdef _WIN32 +#include +#else +#include +#endif struct iio_context_pdata { struct sp_port *port; struct iiod_client *iiod_client; struct iio_context_params params; + + bool shutdown; }; struct iio_buffer_pdata { @@ -145,21 +152,47 @@ static ssize_t serial_write_data(struct iiod_client_pdata *io_data, return ret; } +void sleep_one_ms(void) +{ +#ifdef _WIN32 + Sleep(1); +#else + const struct timespec ts = { + /* One millisecond */ + .tv_nsec = 1 * 1000 * 1000, + }; + + nanosleep(&ts, NULL); +#endif +} + static ssize_t serial_read_data(struct iiod_client_pdata *io_data, char *buf, size_t len, unsigned int timeout_ms) { struct iio_context_pdata *pdata = (struct iio_context_pdata *) io_data; + long long time_left_ms = (long long)timeout_ms; enum sp_return sp_ret; - ssize_t ret; + ssize_t ret = 0; - sp_ret = sp_blocking_read_next(pdata->port, buf, len, timeout_ms); - ret = (ssize_t) libserialport_to_errno(sp_ret); + while (true) { + if (timeout_ms && time_left_ms <= 0) + break; + + sp_ret = sp_nonblocking_read(pdata->port, buf, len); + ret = (ssize_t) libserialport_to_errno(sp_ret); + if (ret || pdata->shutdown) + break; + + sleep_one_ms(); + time_left_ms--; + } prm_dbg(&pdata->params, "Read returned %li: %.*s\n", (long) ret, (int) ret, buf); if (ret == 0) { - prm_err(&pdata->params, "sp_blocking_read_next has timed out\n"); + if (!pdata->shutdown) + prm_err(&pdata->params, "serial_read_data has timed out\n"); return -ETIMEDOUT; } @@ -170,6 +203,8 @@ static void serial_shutdown(struct iio_context *ctx) { struct iio_context_pdata *ctx_pdata = iio_context_get_pdata(ctx); + ctx_pdata->shutdown = true; + iiod_client_destroy(ctx_pdata->iiod_client); sp_close(ctx_pdata->port); sp_free_port(ctx_pdata->port); From a07e10af7d39fa1e9df83cf3603c7b2da8de6549 Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Fri, 1 Dec 2023 16:14:01 +0100 Subject: [PATCH 2/6] serial: Remove dangerous debug print It was using the return value as a specifier for the string length, which is fine as long as the return value is positive, but is most certainly not fine if it is a negative error code. Signed-off-by: Paul Cercueil --- serial.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/serial.c b/serial.c index 4f080b6fa..47099b9ad 100644 --- a/serial.c +++ b/serial.c @@ -187,9 +187,6 @@ static ssize_t serial_read_data(struct iiod_client_pdata *io_data, time_left_ms--; } - prm_dbg(&pdata->params, "Read returned %li: %.*s\n", - (long) ret, (int) ret, buf); - if (ret == 0) { if (!pdata->shutdown) prm_err(&pdata->params, "serial_read_data has timed out\n"); From b9073c7e4354455b38f94090b27165db02cb89f4 Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Fri, 1 Dec 2023 16:11:54 +0100 Subject: [PATCH 3/6] serial: Implement .readbuf / .writebuf This allows Libiio >= v1.0 to talk with IIOD <= v0.25. Signed-off-by: Paul Cercueil --- serial.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/serial.c b/serial.c index 47099b9ad..390b05ae9 100644 --- a/serial.c +++ b/serial.c @@ -262,6 +262,18 @@ static int serial_enable_buffer(struct iio_buffer_pdata *buf, return iiod_client_enable_buffer(buf->pdata, nb_samples, enable); } +static ssize_t +serial_readbuf(struct iio_buffer_pdata *buf, void *dst, size_t len) +{ + return iiod_client_readbuf(buf->pdata, dst, len); +} + +static ssize_t +serial_writebuf(struct iio_buffer_pdata *buf, const void *src, size_t len) +{ + return iiod_client_writebuf(buf->pdata, src, len); +} + static struct iio_block_pdata * serial_create_block(struct iio_buffer_pdata *buf, size_t size, void **data) { @@ -289,6 +301,9 @@ static const struct iio_backend_ops serial_ops = { .free_buffer = serial_free_buffer, .enable_buffer = serial_enable_buffer, + .readbuf = serial_readbuf, + .writebuf = serial_writebuf, + .create_block = serial_create_block, .free_block = iiod_client_free_block, .enqueue_block = iiod_client_enqueue_block, From e7b33e22472d977638bb612d7e8dd92fb10aaa41 Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Fri, 1 Dec 2023 11:27:11 +0100 Subject: [PATCH 4/6] iiod-responder: Handle BINARY command in binary mode Unlike with the network and USB backends, IIOD is unable to detect when a client disconnected. A new client would then send the BINARY command, which would not be understood by IIOD if it already was in binary mode. Use the convenient fact that the BINARY string is exactly the same size as our command opcodes, and compare each opcode read with the "BINARY\r\n" string. In case of a match, simply send a code 0 to tell the client that we are in binary mode, and continue as usual. Signed-off-by: Paul Cercueil --- iiod-responder.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/iiod-responder.c b/iiod-responder.c index 1bd6e0c63..e571ae51b 100644 --- a/iiod-responder.c +++ b/iiod-responder.c @@ -240,12 +240,14 @@ static int iiod_responder_reader_thrd(void *d) { struct iiod_responder *priv = d; struct iiod_command cmd; - struct iiod_buf cmd_buf; + struct iiod_buf cmd_buf, ok_buf; struct iiod_io *io; ssize_t ret = 0; cmd_buf.ptr = &cmd; cmd_buf.size = sizeof(cmd); + ok_buf.ptr = "0\r\n"; + ok_buf.size = 3; iio_mutex_lock(priv->lock); @@ -254,6 +256,18 @@ static int iiod_responder_reader_thrd(void *d) ret = iiod_rw_all(priv, NULL, &cmd_buf, 1, sizeof(cmd), true); + if (!strncmp((char *)&cmd, "BINARY\r\n", 8)) { + /* If we receive again the "BINARY\r\n" string, send a + * return code of zero and continue as usual. + * This can happen with the serial backend when the + * client disconnects and a new client appears. + * Conveniently, the string is exactly 8 bytes, which is + * the size of a iio_command. */ + + iiod_rw_all(priv, NULL, &ok_buf, 1, ok_buf.size, false); + continue; + } + iio_mutex_lock(priv->lock); if (ret <= 0) break; From 4292df5df7c1143d56d6ffcb8252e5d5fa7f46df Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Fri, 1 Dec 2023 14:37:34 +0100 Subject: [PATCH 5/6] compat: Fix behaviour of iio_context_clone() The .clone callback was only implemented by the local and network backends in Libiio <= v0.25. Emulate this behaviour by returning -ENOSYS if using a different backend. Signed-off-by: Paul Cercueil --- compat.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compat.c b/compat.c index cc1358963..f4897aacb 100644 --- a/compat.c +++ b/compat.c @@ -410,6 +410,14 @@ struct iio_context * iio_context_clone(const struct iio_context *old_ctx) uri = IIO_CALL(iio_attr_get_static_value)(attr); params = IIO_CALL(iio_context_get_params)(old_ctx); + if (strncmp(uri, "local:", sizeof("local:") - 1) + && strncmp(uri, "ip:", sizeof("ip:") - 1)) { + /* The .clone callback was only implemented by the local + * and network backends in Libiio <= v0.25. */ + err = -ENOSYS; + goto err_set_errno; + } + ctx = IIO_CALL(iio_create_context)(params, uri); err = iio_err(ctx); if (err) From 40d55148c579ff227b9091dc0d43e562cb2a1e5b Mon Sep 17 00:00:00 2001 From: Paul Cercueil Date: Tue, 5 Dec 2023 11:29:18 +0100 Subject: [PATCH 6/6] task: Fix deadlock between thread and iio_task_stop() It is possible that iio_task_stop() will be called even before the iio_task_run() thread enters the while() loop, where it waits for tasks; in that case, both threads would be waiting on the condition variable without anyone signaling it. Address that issue by signaling the condition variable at the beginning of the for() loop, and not at the end. Signed-off-by: Paul Cercueil --- task.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/task.c b/task.c index d1a5c5258..a5953246d 100644 --- a/task.c +++ b/task.c @@ -50,6 +50,9 @@ static int iio_task_run(void *d) iio_mutex_lock(task->lock); for (;;) { + /* Signal that we're idle */ + iio_cond_signal(task->cond); + while (!task->stop && !(task->list && task->running)) { iio_cond_wait(task->cond, task->lock, 0); @@ -77,9 +80,7 @@ static int iio_task_run(void *d) if (autoclear) iio_task_token_destroy(entry); - /* Signal that we're done with the previous entry */ iio_mutex_lock(task->lock); - iio_cond_signal(task->cond); } iio_mutex_unlock(task->lock);