[gnome-online-accounts/rhel-7.1: 22/34] identity: dramatically simplify alarm logic
- From: Debarshi Ray <debarshir src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-online-accounts/rhel-7.1: 22/34] identity: dramatically simplify alarm logic
- Date: Mon, 4 Aug 2014 13:43:47 +0000 (UTC)
commit 12474127a37d6c4b6be01cb3459ea6246798c1a3
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 34d99f3..c86f665 100644
--- a/src/goaidentity/goaalarm.c
+++ b/src/goaidentity/goaalarm.c
@@ -50,8 +50,6 @@ typedef enum
struct _GoaAlarmPrivate
{
- GCancellable *cancellable;
- gulong cancelled_id;
GDateTime *time;
GDateTime *previous_wakeup_time;
GMainContext *context;
@@ -78,60 +76,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)
- {
- goa_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);
}
@@ -141,8 +102,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);
@@ -161,7 +120,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);
@@ -225,54 +184,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)
{
@@ -341,42 +255,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)
- {
- goa_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)
{
goa_warning ("GoaAlarm: timer source ready callback called "
"when timer source isn't supposed to be used. "
@@ -384,9 +281,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),
@@ -425,7 +319,6 @@ schedule_wakeups_with_timerfd (GoaAlarm *self)
int fd;
int result;
GSource *source;
- GTask *task;
static gboolean seen_before = FALSE;
if (!seen_before)
@@ -458,16 +351,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);
@@ -485,15 +376,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:
@@ -502,7 +389,7 @@ out:
}
static void
-clear_timeout_source_pointer (GoaAlarm *self)
+clear_wakeup_source_pointer (GoaAlarm *self)
{
self->priv->scheduled_wakeup_source = NULL;
}
@@ -536,7 +423,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);
@@ -586,38 +473,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)
@@ -638,11 +499,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 a93991d..75252a4 100644
--- a/src/goaidentity/goaalarm.h
+++ b/src/goaidentity/goaalarm.h
@@ -55,10 +55,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 f06bf30..c3a65f6 100644
--- a/src/goaidentity/goakerberosidentity.c
+++ b/src/goaidentity/goakerberosidentity.c
@@ -56,13 +56,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;
@@ -115,8 +110,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);
@@ -306,9 +299,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
@@ -741,8 +731,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)
{
goa_debug ("GoaKerberosIdentity: renewal alarm fired for signed-in identity");
@@ -767,8 +755,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)
{
goa_debug ("GoaKerberosIdentity: expiring alarm fired for signed-in identity");
@@ -776,25 +762,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);
@@ -803,24 +802,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
@@ -877,15 +887,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);
@@ -894,23 +898,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]