Fix possible threading problems, by using proper class finalizer, and

	* 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.

svn path=/trunk/; revision=1402

diff --git a/gp11/gp11-call.c b/gp11/gp11-call.c
index 26597ea..c9c1ea6 100644 (file)
--- a/gp11/gp11-call.c
+++ b/gp11/gp11-call.c
@@ -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 @@ perform_call (GP11CallFunc func, GCancellable *cancellable, GP11Arguments *args)
 }
 
 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 @@ process_async_call (gpointer data, gpointer unused)
                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 @@ process_result (GP11Call *call, gpointer unused)
        /* 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 @@ process_result (GP11Call *call, gpointer unused)
 }
 
 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 @@ process_completed (void)
 }
 
 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 @@ static void
 _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;
+}
+
+static void
+_gp11_call_base_init (GP11CallClass *klass)
+{
+       GP11CallSource *source;
+       GMainContext *context;
+       GError *err = NULL;
 
-       g_assert (!thread_pool);
-       thread_pool = g_thread_pool_new ((GFunc)process_async_call, NULL, 16, FALSE, &err);
-       if (!thread_pool) {
+       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 (completed_queue) {
-               g_async_queue_unref (completed_queue);
-               completed_queue = NULL;
+
+       if (klass->completed_queue) {
+               g_assert (g_async_queue_length (klass->completed_queue));
+               g_async_queue_unref (klass->completed_queue);
+               klass->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 @@ _gp11_call_async_go (GP11Call *call)
        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 @@ _gp11_call_async_short (GP11Call *call, CK_RV rv)
        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;
 }

diff --git a/gp11/gp11-object.c b/gp11/gp11-object.c
index 1f7eeb6..b7fb1a9 100644 (file)
--- a/gp11/gp11-object.c
+++ b/gp11/gp11-object.c
@@ -329,11 +329,11 @@ void
 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 @@ gp11_object_get_full (GP11Object *object, const gulong *attr_types, gsize n_attr
 
        session = require_session_sync (object, 0, err);
        if (!session)
-               return FALSE;
+               return NULL;
 
        memset (&args, 0, sizeof (args));
        args.attr_types = (gulong*)attr_types;

diff --git a/gp11/gp11-private.h b/gp11/gp11-private.h
index 7fa67cf..743cb57 100644 (file)
--- a/gp11/gp11-private.h
+++ b/gp11/gp11-private.h
@@ -83,34 +83,11 @@ typedef struct _GP11Arguments {
 
 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);
 

diff --git a/gp11/gp11-slot.c b/gp11/gp11-slot.c
index 44518a8..cb50c05 100644 (file)
--- a/gp11/gp11-slot.c
+++ b/gp11/gp11-slot.c
@@ -274,15 +274,12 @@ reuse_session_handle (GP11Session *session, GP11Slot *slot)
         */
        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*

diff --git a/gp11/tests/unit-test-gp11-object.c b/gp11/tests/unit-test-gp11-object.c
index ebc46b9..31e9c6e 100644 (file)
--- a/gp11/tests/unit-test-gp11-object.c
+++ b/gp11/tests/unit-test-gp11-object.c
@@ -383,6 +383,7 @@ DEFINE_TEST(explicit_sessions)
        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);