[gnome-online-accounts] identity: dramatically simplify alarm logic
- From: Ray Strode <halfline src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-online-accounts] identity: dramatically simplify alarm logic
- Date: Mon, 12 May 2014 17:50:07 +0000 (UTC)
commit b2f1b8a70f45848e0d7b88c9c56790a746dd5802
Author: Ray Strode <rstrode redhat com>
Date: Thu May 8 13:31:16 2014 -0400
identity: dramatically simplify alarm logic
The GoaAlarm code is unwieldy. Primarily this is because
it supports resetting the alarm after creation, but can't
clean up old state right away due to a bug in GLib
(bug 705395). All this complexitly is leading to bugs, and
caos.
This commit introduces some zen to the situation by making
GoaAlarm immutable and much simpler. Now to reset an alarm,
the identity code just instantiates a new one and destroys the
old one.
https://bugzilla.gnome.org/show_bug.cgi?id=729718
src/goaidentity/goaalarm.c | 177 ++++-----------------------------
src/goaidentity/goaalarm.h | 5 +-
src/goaidentity/goakerberosidentity.c | 131 ++++++++++++-------------
3 files changed, 82 insertions(+), 231 deletions(-)
---
diff --git a/src/goaidentity/goaalarm.c b/src/goaidentity/goaalarm.c
index dbe66f1..7085229 100644
--- a/src/goaidentity/goaalarm.c
+++ b/src/goaidentity/goaalarm.c
@@ -43,8 +43,6 @@ typedef enum
struct _GoaAlarmPrivate
{
- GCancellable *cancellable;
- gulong cancelled_id;
GDateTime *time;
GDateTime *previous_wakeup_time;
GMainContext *context;
@@ -71,60 +69,23 @@ enum
static void schedule_wakeups (GoaAlarm *self);
static void schedule_wakeups_with_timeout_source (GoaAlarm *self);
+static void goa_alarm_set_time (GoaAlarm *self, GDateTime *time);
+static void clear_wakeup_source_pointer (GoaAlarm *self);
static guint signals[NUMBER_OF_SIGNALS] = { 0 };
G_DEFINE_TYPE (GoaAlarm, goa_alarm, G_TYPE_OBJECT);
static void
-clear_scheduled_immediate_wakeup (GoaAlarm *self)
-{
- g_clear_pointer (&self->priv->immediate_wakeup_source,
- (GDestroyNotify) g_source_destroy);
-}
-
-static void
-clear_scheduled_wakeups (GoaAlarm *self, GSource *source, GInputStream *stream)
-{
- g_rec_mutex_lock (&self->priv->lock);
- clear_scheduled_immediate_wakeup (self);
-
- if (self->priv->type != GOA_ALARM_TYPE_UNSCHEDULED)
- {
- g_source_destroy (source);
-
- if (stream != NULL)
- {
- GError *error;
- gboolean is_closed;
-
- error = NULL;
- is_closed = g_input_stream_close (stream, NULL, &error);
-
- if (!is_closed)
- {
- g_warning ("GoaAlarm: could not close timer stream: %s", error->message);
- g_error_free (error);
- }
-
- g_object_unref (stream);
- }
- }
-
- g_clear_pointer (&self->priv->previous_wakeup_time,
- (GDestroyNotify) g_date_time_unref);
-
- self->priv->type = GOA_ALARM_TYPE_UNSCHEDULED;
- g_rec_mutex_unlock (&self->priv->lock);
-}
-
-static void
goa_alarm_dispose (GObject *object)
{
GoaAlarm *self = GOA_ALARM (object);
- g_clear_object (&self->priv->cancellable);
+ g_clear_object (&self->priv->stream);
+ g_clear_pointer (&self->priv->immediate_wakeup_source, (GDestroyNotify) g_source_destroy);
+ g_clear_pointer (&self->priv->scheduled_wakeup_source, (GDestroyNotify) g_source_destroy);
g_clear_pointer (&self->priv->context, (GDestroyNotify) g_main_context_unref);
g_clear_pointer (&self->priv->time, (GDestroyNotify) g_date_time_unref);
+ g_clear_pointer (&self->priv->previous_wakeup_time, (GDestroyNotify) g_date_time_unref);
G_OBJECT_CLASS (goa_alarm_parent_class)->dispose (object);
}
@@ -134,8 +95,6 @@ goa_alarm_finalize (GObject *object)
{
GoaAlarm *self = GOA_ALARM (object);
- clear_scheduled_wakeups (self, self->priv->scheduled_wakeup_source, self->priv->stream);
-
g_rec_mutex_clear (&self->priv->lock);
G_OBJECT_CLASS (goa_alarm_parent_class)->finalize (object);
@@ -154,7 +113,7 @@ goa_alarm_set_property (GObject *object,
{
case PROP_TIME:
time = (GDateTime *) g_value_get_boxed (value);
- goa_alarm_set_time (self, time, self->priv->cancellable);
+ goa_alarm_set_time (self, time);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, param_spec);
@@ -218,54 +177,9 @@ static void
goa_alarm_init (GoaAlarm *self)
{
self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self, GOA_TYPE_ALARM, GoaAlarmPrivate);
- self->priv->type = GOA_ALARM_TYPE_UNSCHEDULED;
g_rec_mutex_init (&self->priv->lock);
}
-static gboolean
-async_alarm_cancel_idle_cb (gpointer user_data)
-{
- GoaAlarm *self;
- GInputStream *stream;
- GSource *source;
- GTask *task = G_TASK (user_data);
- gpointer task_data;
-
- self = g_task_get_source_object (task);
- source = (GSource *) g_object_get_data (G_OBJECT (task), "alarm-scheduled-wakeup-source");
- task_data = g_object_get_data (G_OBJECT (task), "alarm-stream");
- stream = (task_data == NULL) ? NULL : G_INPUT_STREAM (task_data);
-
- clear_scheduled_wakeups (self, source, stream);
- return G_SOURCE_REMOVE;
-}
-
-static void
-on_cancelled (GCancellable *cancellable, gpointer user_data)
-{
- GoaAlarm *self = GOA_ALARM (user_data);
- GSource *idle_source;
- GTask *task;
-
- task = g_task_new (self, NULL, NULL, NULL);
-
- g_object_set_data_full (G_OBJECT (task),
- "alarm-scheduled-wakeup-source",
- g_source_ref (self->priv->scheduled_wakeup_source),
- (GDestroyNotify) g_source_unref);
-
- if (self->priv->stream != NULL)
- g_object_set_data_full (G_OBJECT (task), "alarm-stream", g_object_ref (self->priv->stream),
g_object_unref);
-
- idle_source = g_idle_source_new ();
- g_source_set_priority (idle_source, G_PRIORITY_HIGH_IDLE);
- g_source_set_callback (idle_source, async_alarm_cancel_idle_cb, g_object_ref (task), g_object_unref);
- g_source_attach (idle_source, self->priv->context);
- g_source_unref (idle_source);
-
- g_object_unref (task);
-}
-
static void
fire_alarm (GoaAlarm *self)
{
@@ -334,42 +248,25 @@ on_immediate_wakeup_source_ready (GoaAlarm *self)
g_return_val_if_fail (self->priv->type != GOA_ALARM_TYPE_UNSCHEDULED, FALSE);
g_rec_mutex_lock (&self->priv->lock);
- if (g_cancellable_is_cancelled (self->priv->cancellable))
- goto out;
-
fire_or_rearm_alarm (self);
-
-out:
g_rec_mutex_unlock (&self->priv->lock);
return FALSE;
}
#ifdef HAVE_TIMERFD
static gboolean
-on_timer_source_ready (GObject *stream, GTask *task)
+on_timer_source_ready (GObject *stream, GoaAlarm *self)
{
gint64 number_of_fires;
gssize bytes_read;
gboolean run_again = FALSE;
GError *error = NULL;
- GoaAlarm *self;
- GCancellable *cancellable;
-
- self = g_task_get_source_object (task);
- cancellable = g_task_get_cancellable (task);
g_return_val_if_fail (GOA_IS_ALARM (self), FALSE);
g_rec_mutex_lock (&self->priv->lock);
- if (self->priv->type == GOA_ALARM_TYPE_UNSCHEDULED)
- {
- g_debug ("GoaAlarm: timer source was unscheduled after "
- "callback was invoked, but before callback got "
- "the lock.");
- goto out;
- }
- else if (self->priv->type != GOA_ALARM_TYPE_TIMER)
+ if (self->priv->type != GOA_ALARM_TYPE_TIMER)
{
g_warning ("GoaAlarm: timer source ready callback called "
"when timer source isn't supposed to be used. "
@@ -377,9 +274,6 @@ on_timer_source_ready (GObject *stream, GTask *task)
goto out;
}
- if (g_cancellable_is_cancelled (cancellable))
- goto out;
-
bytes_read =
g_pollable_input_stream_read_nonblocking (G_POLLABLE_INPUT_STREAM (stream),
&number_of_fires, sizeof (gint64),
@@ -418,7 +312,6 @@ schedule_wakeups_with_timerfd (GoaAlarm *self)
int fd;
int result;
GSource *source;
- GTask *task;
static gboolean seen_before = FALSE;
if (!seen_before)
@@ -451,16 +344,14 @@ schedule_wakeups_with_timerfd (GoaAlarm *self)
self->priv->type = GOA_ALARM_TYPE_TIMER;
self->priv->stream = g_unix_input_stream_new (fd, TRUE);
- task = g_task_new (self, self->priv->cancellable, NULL, NULL);
-
source =
g_pollable_input_stream_create_source (G_POLLABLE_INPUT_STREAM
(self->priv->stream),
- self->priv->cancellable);
+ NULL);
self->priv->scheduled_wakeup_source = source;
g_source_set_callback (self->priv->scheduled_wakeup_source,
- (GSourceFunc) on_timer_source_ready, task,
- (GDestroyNotify) g_object_unref);
+ (GSourceFunc) on_timer_source_ready, self,
+ (GDestroyNotify) clear_wakeup_source_pointer);
g_source_attach (self->priv->scheduled_wakeup_source, self->priv->context);
g_source_unref (source);
@@ -478,15 +369,11 @@ on_timeout_source_ready (GoaAlarm *self)
g_rec_mutex_lock (&self->priv->lock);
- if (g_cancellable_is_cancelled (self->priv->cancellable) ||
- self->priv->type == GOA_ALARM_TYPE_UNSCHEDULED)
+ if (self->priv->type == GOA_ALARM_TYPE_UNSCHEDULED)
goto out;
fire_or_rearm_alarm (self);
- if (g_cancellable_is_cancelled (self->priv->cancellable))
- goto out;
-
schedule_wakeups_with_timeout_source (self);
out:
@@ -495,7 +382,7 @@ out:
}
static void
-clear_timeout_source_pointer (GoaAlarm *self)
+clear_wakeup_source_pointer (GoaAlarm *self)
{
self->priv->scheduled_wakeup_source = NULL;
}
@@ -529,7 +416,7 @@ schedule_wakeups_with_timeout_source (GoaAlarm *self)
g_source_set_callback (self->priv->scheduled_wakeup_source,
(GSourceFunc)
on_timeout_source_ready,
- self, (GDestroyNotify) clear_timeout_source_pointer);
+ self, (GDestroyNotify) clear_wakeup_source_pointer);
g_source_attach (self->priv->scheduled_wakeup_source, self->priv->context);
g_source_unref (source);
@@ -579,38 +466,12 @@ schedule_immediate_wakeup (GoaAlarm *self)
g_source_unref (source);
}
-void
-goa_alarm_set_time (GoaAlarm *self, GDateTime *time, GCancellable *cancellable)
+static void
+goa_alarm_set_time (GoaAlarm *self, GDateTime *time)
{
- if (g_cancellable_is_cancelled (cancellable))
- return;
-
g_rec_mutex_lock (&self->priv->lock);
- if (self->priv->cancellable != NULL && self->priv->cancellable != cancellable)
- g_cancellable_cancel (self->priv->cancellable);
-
- if (cancellable != NULL)
- g_object_ref (cancellable);
-
- if (self->priv->cancelled_id != 0)
- g_cancellable_disconnect (self->priv->cancellable, self->priv->cancelled_id);
-
- g_clear_object (&self->priv->cancellable);
-
- if (cancellable != NULL)
- self->priv->cancellable = cancellable;
- else
- self->priv->cancellable = g_cancellable_new ();
-
- self->priv->cancelled_id = g_cancellable_connect (self->priv->cancellable,
- G_CALLBACK (on_cancelled),
- self, NULL);
g_date_time_ref (time);
-
- if (self->priv->time != NULL)
- g_date_time_unref (self->priv->time);
-
self->priv->time = time;
if (self->priv->context == NULL)
@@ -631,11 +492,11 @@ goa_alarm_get_time (GoaAlarm *self)
}
GoaAlarm *
-goa_alarm_new (void)
+goa_alarm_new (GDateTime *alarm_time)
{
GoaAlarm *self;
- self = GOA_ALARM (g_object_new (GOA_TYPE_ALARM, NULL));
+ self = GOA_ALARM (g_object_new (GOA_TYPE_ALARM, "time", alarm_time, NULL));
return GOA_ALARM (self);
}
diff --git a/src/goaidentity/goaalarm.h b/src/goaidentity/goaalarm.h
index 7ef4bde..5f15033 100644
--- a/src/goaidentity/goaalarm.h
+++ b/src/goaidentity/goaalarm.h
@@ -51,10 +51,7 @@ struct _GoaAlarmClass
GType goa_alarm_get_type (void);
-GoaAlarm *goa_alarm_new (void);
-void goa_alarm_set_time (GoaAlarm *alarm,
- GDateTime *time,
- GCancellable *cancellable);
+GoaAlarm *goa_alarm_new (GDateTime *time);
GDateTime *goa_alarm_get_time (GoaAlarm *alarm);
G_END_DECLS
#endif /* __GOA_ALARM_H__ */
diff --git a/src/goaidentity/goakerberosidentity.c b/src/goaidentity/goakerberosidentity.c
index 1596a5f..ab86d36 100644
--- a/src/goaidentity/goakerberosidentity.c
+++ b/src/goaidentity/goakerberosidentity.c
@@ -51,13 +51,8 @@ struct _GoaKerberosIdentityPrivate
guint expiration_time_idle_id;
GoaAlarm *expiration_alarm;
- GCancellable *expiration_alarm_cancellable;
-
GoaAlarm *expiring_alarm;
- GCancellable *expiring_alarm_cancellable;
-
GoaAlarm *renewal_alarm;
- GCancellable *renewal_alarm_cancellable;
VerificationLevel cached_verification_level;
guint is_signed_in_idle_id;
@@ -110,8 +105,6 @@ goa_kerberos_identity_dispose (GObject *object)
GoaKerberosIdentity *self = GOA_KERBEROS_IDENTITY (object);
G_LOCK (identity_lock);
- clear_alarms (self);
-
g_clear_object (&self->priv->renewal_alarm);
g_clear_object (&self->priv->expiring_alarm);
g_clear_object (&self->priv->expiration_alarm);
@@ -301,9 +294,6 @@ goa_kerberos_identity_init (GoaKerberosIdentity *self)
self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
GOA_TYPE_KERBEROS_IDENTITY,
GoaKerberosIdentityPrivate);
- self->priv->expiration_alarm = goa_alarm_new ();
- self->priv->expiring_alarm = goa_alarm_new ();
- self->priv->renewal_alarm = goa_alarm_new ();
}
static void
@@ -736,8 +726,6 @@ on_renewal_alarm_fired (GoaAlarm *alarm,
g_return_if_fail (GOA_IS_ALARM (alarm));
g_return_if_fail (GOA_IS_KERBEROS_IDENTITY (self));
- g_clear_object (&self->priv->renewal_alarm_cancellable);
-
if (self->priv->cached_verification_level == VERIFICATION_LEVEL_SIGNED_IN)
{
g_debug ("GoaKerberosIdentity: renewal alarm fired for signed-in identity");
@@ -762,8 +750,6 @@ on_expiring_alarm_fired (GoaAlarm *alarm,
g_return_if_fail (GOA_IS_ALARM (alarm));
g_return_if_fail (GOA_IS_KERBEROS_IDENTITY (self));
- g_clear_object (&self->priv->expiring_alarm_cancellable);
-
if (self->priv->cached_verification_level == VERIFICATION_LEVEL_SIGNED_IN)
{
g_debug ("GoaKerberosIdentity: expiring alarm fired for signed-in identity");
@@ -771,25 +757,38 @@ on_expiring_alarm_fired (GoaAlarm *alarm,
}
}
+static gboolean
+unref_alarm (GoaAlarm *alarm)
+{
+ g_object_unref (G_OBJECT (alarm));
+ return G_SOURCE_REMOVE;
+}
+
static void
-set_alarm (GoaKerberosIdentity *self,
- GoaAlarm *alarm,
- GDateTime *alarm_time,
- GCancellable **cancellable)
+clear_alarm_and_unref_on_idle (GoaKerberosIdentity *self,
+ GoaAlarm **alarm)
{
- GDateTime *old_alarm_time;
+ if (!*alarm)
+ return;
+
+ g_idle_add ((GSourceFunc) unref_alarm, *alarm);
+ *alarm = NULL;
+}
+
+static void
+reset_alarm (GoaKerberosIdentity *self,
+ GoaAlarm **alarm,
+ GDateTime *alarm_time)
+{
+ GDateTime *old_alarm_time = NULL;
G_LOCK (identity_lock);
- old_alarm_time = goa_alarm_get_time (alarm);
+ if (*alarm)
+ old_alarm_time = goa_alarm_get_time (*alarm);
if (old_alarm_time == NULL || !g_date_time_equal (alarm_time, old_alarm_time))
{
- GCancellable *new_cancellable;
-
- new_cancellable = g_cancellable_new ();
- goa_alarm_set_time (alarm, alarm_time, new_cancellable);
-
- g_clear_object (cancellable);
- *cancellable = new_cancellable;
+ clear_alarm_and_unref_on_idle (self, alarm);
+ *alarm = goa_alarm_new (alarm_time);
}
G_UNLOCK (identity_lock);
@@ -798,24 +797,35 @@ set_alarm (GoaKerberosIdentity *self,
static void
disconnect_alarm_signals (GoaKerberosIdentity *self)
{
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->renewal_alarm),
- G_CALLBACK (on_renewal_alarm_fired),
- self);
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->renewal_alarm),
- G_CALLBACK (on_renewal_alarm_rearmed),
- self);
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiring_alarm),
- G_CALLBACK (on_expiring_alarm_fired),
- self);
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiration_alarm),
- G_CALLBACK (on_expiration_alarm_rearmed),
- self);
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiration_alarm),
- G_CALLBACK (on_expiration_alarm_fired),
- self);
- g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiring_alarm),
- G_CALLBACK (on_expiring_alarm_rearmed),
- self);
+ if (self->priv->renewal_alarm)
+ {
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->renewal_alarm),
+ G_CALLBACK (on_renewal_alarm_fired),
+ self);
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->renewal_alarm),
+ G_CALLBACK (on_renewal_alarm_rearmed),
+ self);
+ }
+
+ if (self->priv->expiring_alarm)
+ {
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiring_alarm),
+ G_CALLBACK (on_expiring_alarm_fired),
+ self);
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiring_alarm),
+ G_CALLBACK (on_expiring_alarm_rearmed),
+ self);
+ }
+
+ if (self->priv->expiration_alarm)
+ {
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiration_alarm),
+ G_CALLBACK (on_expiration_alarm_rearmed),
+ self);
+ g_signal_handlers_disconnect_by_func (G_OBJECT (self->priv->expiration_alarm),
+ G_CALLBACK (on_expiration_alarm_fired),
+ self);
+ }
}
static void
@@ -872,15 +882,9 @@ reset_alarms (GoaKerberosIdentity *self)
disconnect_alarm_signals (self);
- set_alarm (self,
- self->priv->renewal_alarm,
- renewal_time, &self->priv->renewal_alarm_cancellable);
- set_alarm (self,
- self->priv->expiring_alarm,
- expiring_time, &self->priv->expiring_alarm_cancellable);
- set_alarm (self,
- self->priv->expiration_alarm,
- expiration_time, &self->priv->expiration_alarm_cancellable);
+ reset_alarm (self, &self->priv->renewal_alarm, renewal_time);
+ reset_alarm (self, &self->priv->expiring_alarm, expiring_time);
+ reset_alarm (self, &self->priv->expiration_alarm, expiration_time);
g_date_time_unref (renewal_time);
g_date_time_unref (expiring_time);
@@ -889,23 +893,12 @@ reset_alarms (GoaKerberosIdentity *self)
}
static void
-cancel_and_clear_cancellable (GCancellable **cancellable)
-{
- if (cancellable == NULL)
- return;
-
- if (!g_cancellable_is_cancelled (*cancellable))
- g_cancellable_cancel (*cancellable);
-
- g_clear_object (cancellable);
-}
-
-static void
clear_alarms (GoaKerberosIdentity *self)
{
- cancel_and_clear_cancellable (&self->priv->renewal_alarm_cancellable);
- cancel_and_clear_cancellable (&self->priv->expiring_alarm_cancellable);
- cancel_and_clear_cancellable (&self->priv->expiration_alarm_cancellable);
+ disconnect_alarm_signals (self);
+ clear_alarm_and_unref_on_idle (self, &self->priv->renewal_alarm);
+ clear_alarm_and_unref_on_idle (self, &self->priv->expiring_alarm);
+ clear_alarm_and_unref_on_idle (self, &self->priv->expiration_alarm);
}
static gboolean
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]