gnome-keyring r1402 - in trunk: . gp11 gp11/tests

[nnielsen svn gnome org]

Author: nnielsen
Date: Sun Dec 21 17:15:37 2008
New Revision: 1402
URL: http://svn.gnome.org/viewvc/gnome-keyring?rev=1402&view=rev

Log:
	* gp11/gp11-private.h: 
	* gp11/gp11-call.c: Fix possible threading problems, by using 
	proper class finalizer, and hiding all instance details.
	
	* gp11/gp11-object.c: Fix possible reference counting problem.
	
	* gp11/gp11-slot.c: 
	* gp11/tests/unit-test-gp11-object.c: Fix test reference problems. 

Modified:
   trunk/ChangeLog
   trunk/gp11/gp11-call.c
   trunk/gp11/gp11-object.c
   trunk/gp11/gp11-private.h
   trunk/gp11/gp11-slot.c
   trunk/gp11/tests/unit-test-gp11-object.c

Modified: trunk/gp11/gp11-call.c
==============================================================================
--- trunk/gp11/gp11-call.c	(original)
+++ trunk/gp11/gp11-call.c	Sun Dec 21 17:15:37 2008
@@ -27,14 +27,41 @@
 
 #include <string.h>
 
-static GThreadPool *thread_pool = NULL;
-static GAsyncQueue *completed_queue = NULL;
-static guint completed_id = 0;
+typedef struct _GP11CallSource GP11CallSource;
 
-static void _gp11_call_implement_async_result (GAsyncResultIface *iface);
+static gpointer _gp11_call_parent_class = NULL;
 
-G_DEFINE_TYPE_EXTENDED (GP11Call, _gp11_call, G_TYPE_OBJECT, 0,
-        G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_RESULT, _gp11_call_implement_async_result));
+struct _GP11Call {
+	GObject parent;
+	
+	/* For making the call */
+	GP11CallFunc func;
+	GP11Arguments *args;
+	GCancellable *cancellable;
+	GDestroyNotify destroy;
+	CK_RV rv;
+	
+	/* For result callback only */
+	gpointer object;
+	GAsyncReadyCallback callback;
+	gpointer user_data;
+	
+	/* For authenticating */
+	gboolean do_login;
+	gchar *password;
+};
+
+struct _GP11CallClass {
+	GObjectClass parent;
+	GThreadPool *thread_pool;
+	GAsyncQueue *completed_queue;
+	guint completed_id;
+};
+
+struct _GP11CallSource {
+	GSource source;
+	GP11CallClass *klass;
+};
 
 /* ----------------------------------------------------------------------------
  * HELPER FUNCTIONS
@@ -70,7 +97,7 @@
 }
 
 static void
-process_async_call (gpointer data, gpointer unused)
+process_async_call (gpointer data, GP11CallClass *klass)
 {
 	GP11Call *call = GP11_CALL (data);
 	CK_ULONG pin_len;
@@ -95,11 +122,12 @@
 		call->rv = perform_call (call->func, call->cancellable, call->args);
 	}
 	
-	g_async_queue_push (completed_queue, call);
+	g_async_queue_push (klass->completed_queue, call);
 	
 	/* Wakeup main thread if on a separate thread */
 	g_main_context_wakeup (NULL);
 }
+
 static void 
 process_result (GP11Call *call, gpointer unused)
 {
@@ -126,7 +154,7 @@
 	/* If we're supposed to do a login, then queue this call again */
 	if (call->do_login) {
 		g_object_ref (call);
-		g_thread_pool_push (thread_pool, call, NULL);
+		g_thread_pool_push (GP11_CALL_GET_CLASS (call)->thread_pool, call, NULL);
 		return;
 	}
 	
@@ -139,13 +167,13 @@
 }
 
 static gboolean
-process_completed (void)
+process_completed (GP11CallClass *klass)
 {
 	gpointer call;
 
-	g_assert (completed_queue);
+	g_assert (klass->completed_queue);
 
-	call = g_async_queue_try_pop (completed_queue);
+	call = g_async_queue_try_pop (klass->completed_queue);
 	if (call) {
 		process_result (call, NULL);
 		g_object_unref (call);
@@ -156,31 +184,35 @@
 }
 
 static gboolean
-completed_prepare(GSource* source, gint *timeout)
+completed_prepare(GSource* base, gint *timeout)
 {
+	GP11CallSource *source = (GP11CallSource*)base;
 	gboolean have;
-	g_assert (completed_queue);
-	have = g_async_queue_length (completed_queue) > 0;
+	
+	g_assert (source->klass->completed_queue);
+	have = g_async_queue_length (source->klass->completed_queue) > 0;
 	*timeout = have ? 0 : -1;
 	return have;
 }
 
 static gboolean
-completed_check(GSource* source)
+completed_check(GSource* base)
 {
-	g_assert (completed_queue);
-	return g_async_queue_length (completed_queue) > 0;
+	GP11CallSource *source = (GP11CallSource*)base;
+	g_assert (source->klass->completed_queue);
+	return g_async_queue_length (source->klass->completed_queue) > 0;
 }
 
 static gboolean
-completed_dispatch(GSource* source, GSourceFunc callback, gpointer user_data)
+completed_dispatch(GSource* base, GSourceFunc callback, gpointer user_data)
 {
-	process_completed ();
+	GP11CallSource *source = (GP11CallSource*)base;
+	process_completed (source->klass);
 	return TRUE;
 }
 
 static void
-completed_finalize(GSource* source)
+completed_finalize(GSource* base)
 {
 	
 }
@@ -252,64 +284,108 @@
 _gp11_call_class_init (GP11CallClass *klass)
 {
 	GObjectClass *gobject_class = (GObjectClass*)klass;
-	GMainContext *context;
-	GError *err = NULL;
-	GSource *src;
 
 	_gp11_call_parent_class = g_type_class_peek_parent (klass);
 	gobject_class->finalize = _gp11_call_finalize;
-	
-	g_assert (!thread_pool);
-	thread_pool = g_thread_pool_new ((GFunc)process_async_call, NULL, 16, FALSE, &err);
-	if (!thread_pool) {
+}
+
+static void
+_gp11_call_base_init (GP11CallClass *klass)
+{
+	GP11CallSource *source;
+	GMainContext *context;
+	GError *err = NULL;
+
+	klass->thread_pool = g_thread_pool_new ((GFunc)process_async_call, klass, 16, FALSE, &err);
+	if (!klass->thread_pool) {
 		g_critical ("couldn't create thread pool: %s", 
 		            err && err->message ? err->message : "");
 		return;
 	}
 	
-	g_assert (!completed_queue);
-	completed_queue = g_async_queue_new_full (g_object_unref);
-	g_assert (completed_queue);
+	klass->completed_queue = g_async_queue_new_full (g_object_unref);
+	g_assert (klass->completed_queue);
 	
 	context = g_main_context_default ();
 	g_assert (context);
 	
 	/* Add our idle handler which processes other tasks */
-	g_assert (!completed_id);
-	src = g_source_new (&completed_functions, sizeof (GSource));
-	completed_id = g_source_attach (src, context);
-	g_source_set_callback (src, NULL, NULL, NULL);
-	g_source_unref (src);
+	source = (GP11CallSource*)g_source_new (&completed_functions, sizeof (GP11CallSource));
+	source->klass = klass;
+	klass->completed_id = g_source_attach ((GSource*)source, context);
+	g_source_set_callback ((GSource*)source, NULL, NULL, NULL);
+	g_source_unref ((GSource*)source);
 }
 
-/* ----------------------------------------------------------------------------
- * PUBLIC
- */
-
-void
-_gp11_call_uninitialize (void)
+static void
+_gp11_call_base_finalize (GP11CallClass *klass)
 {
 	GMainContext *context;
 	GSource *src;
 
-	if (thread_pool) {
-		g_thread_pool_free (thread_pool, FALSE, TRUE);
-		thread_pool = NULL;
+	if (klass->thread_pool) {
+		g_assert (g_thread_pool_unprocessed (klass->thread_pool) == 0);
+		g_thread_pool_free (klass->thread_pool, FALSE, TRUE);
+		klass->thread_pool = NULL;
 	}
 	
-	if (completed_id) {
+	if (klass->completed_id) {
 		context = g_main_context_default ();
 		g_return_if_fail (context);
 		
-		src = g_main_context_find_source_by_id (context, completed_id);
+		src = g_main_context_find_source_by_id (context, klass->completed_id);
 		g_assert (src);
 		g_source_destroy (src);
-		completed_id = 0;
+		klass->completed_id = 0;
+	}
+	
+	if (klass->completed_queue) {
+		g_assert (g_async_queue_length (klass->completed_queue));
+		g_async_queue_unref (klass->completed_queue);
+		klass->completed_queue = NULL;
 	}
-	if (completed_queue) {
-		g_async_queue_unref (completed_queue);
-		completed_queue = NULL;
+}
+
+GType
+_gp11_call_get_type (void) 
+{
+	static volatile gsize type_id__volatile = 0;
+	
+	if (g_once_init_enter (&type_id__volatile)) {
+		
+		static const GTypeInfo type_info = {
+			sizeof (GP11CallClass), 
+			(GBaseInitFunc)_gp11_call_base_init, 
+			(GBaseFinalizeFunc)_gp11_call_base_finalize, 
+			(GClassInitFunc)_gp11_call_class_init, 
+			(GClassFinalizeFunc)NULL, 
+			NULL,   // class_data 
+			sizeof (GP11Call), 
+			0,      // n_preallocs 
+			(GInstanceInitFunc)_gp11_call_init, 
+		};
+
+		static const GInterfaceInfo interface_info = {
+			(GInterfaceInitFunc)_gp11_call_implement_async_result
+		};
+
+		GType type_id = g_type_register_static (G_TYPE_OBJECT, "_GP11Call", &type_info, 0); 
+		g_type_add_interface_static (type_id, G_TYPE_ASYNC_RESULT, &interface_info);
+
+		g_once_init_leave (&type_id__volatile, type_id);
 	}
+	
+	return type_id__volatile;
+}
+
+/* ----------------------------------------------------------------------------
+ * PUBLIC
+ */
+
+void
+_gp11_call_uninitialize (void)
+{
+
 }
 
 gboolean
@@ -438,10 +514,10 @@
 	g_assert (call->args->pkcs11);
 
 	/* To keep things balanced, process at one completed event */
-	process_completed();
+	process_completed(GP11_CALL_GET_CLASS (call));
 
-	g_assert (thread_pool);
-	g_thread_pool_push (thread_pool, call, NULL);
+	g_assert (GP11_CALL_GET_CLASS (call)->thread_pool);
+	g_thread_pool_push (GP11_CALL_GET_CLASS (call)->thread_pool, call, NULL);
 }
 
 void
@@ -475,6 +551,13 @@
 	call->rv = rv;
 
 	/* Already complete, so just push it for processing in main loop */
-	g_assert (completed_queue);
-	g_async_queue_push (completed_queue, call);
+	g_assert (GP11_CALL_GET_CLASS (call)->completed_queue);
+	g_async_queue_push (GP11_CALL_GET_CLASS (call)->completed_queue, call);
+}
+
+gpointer
+_gp11_call_get_arguments (GP11Call *call)
+{
+	g_assert (GP11_IS_CALL (call));
+	return call->args;
 }

Modified: trunk/gp11/gp11-object.c
==============================================================================
--- trunk/gp11/gp11-object.c	(original)
+++ trunk/gp11/gp11-object.c	Sun Dec 21 17:15:37 2008
@@ -329,11 +329,11 @@
 gp11_object_set_session (GP11Object *object, GP11Session *session)
 {
 	g_return_if_fail (GP11_IS_OBJECT (object));
+	if (session)
+		g_object_ref (session);
 	if (object->session)
 		g_object_unref (object->session);
 	object->session = session;
-	if (object->session)
-		g_object_ref (object->session);
 }
 
 /* DESTROY */
@@ -744,7 +744,7 @@
 	
 	session = require_session_sync (object, 0, err);
 	if (!session)
-		return FALSE;
+		return NULL;
 	
 	memset (&args, 0, sizeof (args));
 	args.attr_types = (gulong*)attr_types;

Modified: trunk/gp11/gp11-private.h
==============================================================================
--- trunk/gp11/gp11-private.h	(original)
+++ trunk/gp11/gp11-private.h	Sun Dec 21 17:15:37 2008
@@ -83,34 +83,11 @@
 
 typedef struct _GP11CallClass GP11CallClass;
 
-struct _GP11Call {
-	GObject parent;
-	
-	/* For making the call */
-	GP11CallFunc func;
-	GP11Arguments *args;
-	GCancellable *cancellable;
-	GDestroyNotify destroy;
-	CK_RV rv;
-	
-	/* For result callback only */
-	gpointer object;
-	GAsyncReadyCallback callback;
-	gpointer user_data;
-	
-	/* For authenticating */
-	gboolean do_login;
-	gchar *password;
-};
-
-struct _GP11CallClass {
-	GObjectClass parent;
-};
-
 GType              _gp11_call_get_type                    (void) G_GNUC_CONST;
 
-#define            _gp11_call_arguments(call, type) \
-			(type*)(GP11_CALL (call)->args)
+#define            _gp11_call_arguments(call, type)       (type*)(_gp11_call_get_arguments (GP11_CALL (call)))
+
+gpointer           _gp11_call_get_arguments               (GP11Call *call);
 
 void               _gp11_call_uninitialize                (void);
 

Modified: trunk/gp11/gp11-slot.c
==============================================================================
--- trunk/gp11/gp11-slot.c	(original)
+++ trunk/gp11/gp11-slot.c	Sun Dec 21 17:15:37 2008
@@ -274,15 +274,12 @@
 	 */
 	flags = g_object_get_data (G_OBJECT (session), "gp11-open-session-flags");
 	g_return_if_fail (flags);
-	if ((*flags & info.flags) != *flags) {
-g_message ("discarding session, wrong flags");
+	if ((*flags & info.flags) != *flags)
 		return;
-	}
 	
 	/* Keep this one around for later use */
 	push_session_table (slot, *flags, session->handle);
 	session->handle = 0;
-g_message ("keeping the session for reuse");
 }
 
 static GP11Session*

Modified: trunk/gp11/tests/unit-test-gp11-object.c
==============================================================================
--- trunk/gp11/tests/unit-test-gp11-object.c	(original)
+++ trunk/gp11/tests/unit-test-gp11-object.c	Sun Dec 21 17:15:37 2008
@@ -377,13 +377,14 @@
 	gulong klass;
 	gchar *value = NULL;
 	gulong types[2] = { CKA_CLASS, CKA_LABEL };
-	
+
 	/* Set an explicit session */
 	gp11_object_set_session (object, session);
 	g_assert (gp11_object_get_session (object) == session);
 	g_object_get (object, "session", &sess, NULL);
 	g_assert (sess == session);
-	
+	g_object_unref (sess);
+
 	/* Simple */
 	attrs = gp11_object_get (object, &err, CKA_CLASS, CKA_LABEL, -1);
 	SUCCESS_RES (attrs, err);