session: Make sr_session_stop thread-safe

With the sigrok session running in a worker thread, if sr_session_stop is called
from another thread, it shuts down the pollfds used by the hardware drivers,
without ensuring that the sigrok event loop is no longer using those pollfds.

On the demo driver, this involves shutting down the GIOChannels, causing a
segfault when the sigrok event loop tries to use them. This is evident when
using the Stop button in PulseView, while the session is running.

This isn't a problem with just the demo driver; any driver's resources may be
freed by sr_session_stop concurrently with the sigrok session running.

To solve this problem, we don't touch the session itself in sr_session_stop().
Instead, we mark it for decommissioning and return. The session polls this flag,
and shuts itself down when requested.

This fixes bug 4.

Signed-off-by: Alexandru Gagniuc <redacted>

diff --git a/libsigrok-internal.h b/libsigrok-internal.h
index b8b2adc0a7df9fc20bc22baff66a88c29f7479e1..bd1b7289e979d3b760c819bec1356b25fe638ae9 100644 (file)
--- a/libsigrok-internal.h
+++ b/libsigrok-internal.h
@@ -123,6 +123,7 @@ SR_PRIV int sr_source_add(int fd, int events, int timeout,
 
 SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi,
                            const struct sr_datafeed_packet *packet);
+SR_PRIV int sr_session_stop_sync(void);
 
 /*--- std.c -----------------------------------------------------------------*/
 

diff --git a/libsigrok.h b/libsigrok.h
index fba6c9ccf2910c2c86f342480dcf4ec1aba2907b..4212c59c4f046d77522b81a12b9a949568664123 100644 (file)
--- a/libsigrok.h
+++ b/libsigrok.h
@@ -775,6 +775,14 @@ struct sr_session {
        struct source *sources;
        GPollFD *pollfds;
        int source_timeout;
+
+       /*
+        * These are our synchronization primitives for stopping the session in
+        * an async fashion. We need to make sure the session is stopped from
+        * within the session thread itself.
+        */
+       GMutex stop_mutex;
+       gboolean abort_session;
 };
 
 #include "proto.h"

diff --git a/session.c b/session.c
index ac5981ad050b6993478ca04f0838345c704e2eb8..acf1158b4e3ad0e75061c4aa9ab831f5c5aadbd1 100644 (file)
--- a/session.c
+++ b/session.c
@@ -79,6 +79,8 @@ SR_API struct sr_session *sr_session_new(void)
        }
 
        session->source_timeout = -1;
+       session->abort_session = FALSE;
+       g_mutex_init(&session->stop_mutex);
 
        return session;
 }
@@ -101,6 +103,8 @@ SR_API int sr_session_destroy(void)
 
        /* TODO: Error checks needed? */
 
+       g_mutex_clear(&session->stop_mutex);
+
        g_free(session);
        session = NULL;
 
@@ -283,6 +287,16 @@ static int sr_session_run_poll(void)
                                                session->sources[i].cb_data))
                                        sr_session_source_remove(session->sources[i].poll_object);
                        }
+                       /*
+                        * We want to take as little time as possible to stop
+                        * the session if we have been told to do so. Therefore,
+                        * we check the flag after processing every source, not
+                        * just once per main event loop.
+                        */
+                       g_mutex_lock(&session->stop_mutex);
+                       if (session->abort_session)
+                               sr_session_stop_sync();
+                       g_mutex_unlock(&session->stop_mutex);
                }
        }
 
@@ -372,9 +386,12 @@ SR_API int sr_session_run(void)
  * The current session is stopped immediately, with all acquisition sessions
  * being stopped and hardware drivers cleaned up.
  *
+ * This must be called from within the session thread, to prevent freeing
+ * resources that the session thread will try to use.
+ *
  * @return SR_OK upon success, SR_ERR_BUG if no session exists.
  */
-SR_API int sr_session_stop(void)
+SR_PRIV int sr_session_stop_sync(void)
 {
        struct sr_dev_inst *sdi;
        GSList *l;
@@ -406,6 +423,33 @@ SR_API int sr_session_stop(void)
        return SR_OK;
 }
 
+/**
+ * Stop the current session.
+ *
+ * The current session is stopped immediately, with all acquisition sessions
+ * being stopped and hardware drivers cleaned up.
+ *
+ * If the session is run in a separate thread, this function will not block
+ * until the session is finished executing. It is the caller's responsibility
+ * to wait for the session thread to return before assuming that the session is
+ * completely decommissioned.
+ *
+ * @return SR_OK upon success, SR_ERR_BUG if no session exists.
+ */
+SR_API int sr_session_stop(void)
+{
+       if (!session) {
+               sr_err("%s: session was NULL", __func__);
+               return SR_ERR_BUG;
+       }
+
+       g_mutex_lock(&session->stop_mutex);
+       session->abort_session = TRUE;
+       g_mutex_unlock(&session->stop_mutex);
+
+       return SR_OK;
+}
+
 /**
  * Debug helper.
  *