From: Alexandru Gagniuc <redacted>
Date: Sat, 2 Feb 2013 06:50:00 +0000 (-0600)
Subject: session: Make sr_session_stop thread-safe
X-Git-Tag: dsupstream~154
X-Git-Url: http://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=33c6e4c5a428115965f980e88e6415fb782658e9

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 b8b2adc0..bd1b7289 100644
--- 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 fba6c9cc..4212c59c 100644
--- 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 ac5981ad..acf1158b 100644
--- 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.
  *