Indicator Date and Time

Merge lp:~charlesk/indicator-datetime/lp-1424966-fix-ecanceled-errno-rtm-14.09 into lp:indicator-datetime/rtm-14.09

  1. lp-1424966-fix-ecanceled-errno-rtm-14.09
  2. Merge into trunk.rtm-14.09
Proposed by Charles Kerr
Status: Merged
Approved by: Ted Gould
Approved revision: 397
Merged at revision: 397
Proposed branch: lp:~charlesk/indicator-datetime/lp-1424966-fix-ecanceled-errno-rtm-14.09
Merge into: lp:indicator-datetime/rtm-14.09
Diff against target: 97 lines (+35/-24)
1 file modified
src/clock-live.cpp (+35/-24)
To merge this branch: bzr merge lp:~charlesk/indicator-datetime/lp-1424966-fix-ecanceled-errno-rtm-14.09
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+250871@code.launchpad.net

Commit message

Avoid ECANCELED errors in calls to timerfd_settime()

Description of the change

=== Change Description

timerfd_settime() can fail with ECANCELED if the system's monotonic offset has changed since the timer was created with timerfd_create(). This patch avoids that case by using a new timerfd for each call to timerfd_settime().

* Extract the timer+poll cleanup code from Impl::~Impl into new private method unset_timer() called by ~Impl() and now also by reset_timer() as part of replacing the old timerfd with a new one

* Moved timerfd_create() and g_unix_fd_add() calls from Impl::Impl() into reset_timer() to complete the process of replacing the old timerfd with a new one

* Demoted condition logging from a g_message() to a g_debug() because it happens too frequently for a g_message()

=== Checklist

> Are there any related MPs required for this MP to build/function as expected? Please list.

No

> Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Yes

> Did the code build without warnings?

Yes

> Did the tests run successfully?

Yes

> Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> What device (or emulator) has your component test plan been executed successfully on?

krillin 14.10 r244

> What manual tests are relevant for this MP?

indicator-datetime/manual-time

> Did you include a link to the MR Review Checklist Template to make your reviewer's life easier?

https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-datetime

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

Okay, this makes a lot more sense now that I realize it isn't for alarms. :-) My fault.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/clock-live.cpp'
2--- src/clock-live.cpp 2015-02-13 20:27:16 +0000
3+++ src/clock-live.cpp 2015-02-25 05:19:41 +0000
4@@ -52,31 +52,13 @@
5 setter(m_timezone->timezone.get());
6 }
7
8- m_timerfd = timerfd_create(CLOCK_REALTIME, 0);
9- if (m_timerfd == -1)
10- {
11- g_warning("unable to create realtime timer: %s", g_strerror(errno));
12- }
13- else
14- {
15- reset_timer();
16-
17- m_timerfd_tag = g_unix_fd_add(m_timerfd,
18- (GIOCondition)(G_IO_IN|G_IO_HUP|G_IO_ERR),
19- on_timerfd_cond,
20- this);
21- }
22-
23+ reset_timer();
24 refresh();
25 }
26
27 ~Impl()
28 {
29- if (m_timerfd_tag != 0)
30- g_source_remove(m_timerfd_tag);
31-
32- if (m_timerfd != -1)
33- close(m_timerfd);
34+ unset_timer();
35
36 g_clear_pointer(&m_gtimezone, g_time_zone_unref);
37 }
38@@ -93,10 +75,33 @@
39
40 private:
41
42+ void unset_timer()
43+ {
44+ if (m_timerfd_tag != 0)
45+ {
46+ g_source_remove(m_timerfd_tag);
47+ m_timerfd_tag = 0;
48+ }
49+
50+ if (m_timerfd != -1)
51+ {
52+ close(m_timerfd);
53+ m_timerfd = -1;
54+ }
55+ }
56+
57 void reset_timer()
58 {
59+ // clear out any previous timer
60+ unset_timer();
61+
62+ // create a new timer
63+ m_timerfd = timerfd_create(CLOCK_REALTIME, 0);
64+ if (m_timerfd == -1)
65+ g_error("unable to create realtime timer: %s", g_strerror(errno));
66+
67+ // set args to fire at the beginning of the next minute...
68 struct itimerspec timerval;
69- // set args to fire at the beginning of the next minute...
70 int flags = TFD_TIMER_ABSTIME;
71 auto now = g_date_time_new_now(m_gtimezone);
72 auto next = g_date_time_add_minutes(now, 1);
73@@ -115,6 +120,12 @@
74
75 if (timerfd_settime(m_timerfd, flags, &timerval, NULL) == -1)
76 g_error("timerfd_settime failed: %s", g_strerror(errno));
77+
78+ // listen for the changes/timers
79+ m_timerfd_tag = g_unix_fd_add(m_timerfd,
80+ (GIOCondition)(G_IO_IN|G_IO_HUP|G_IO_ERR),
81+ on_timerfd_cond,
82+ this);
83 }
84
85 static gboolean on_timerfd_cond (gint fd, GIOCondition cond, gpointer gself)
86@@ -130,9 +141,9 @@
87 {
88 auto now = g_date_time_new_now(self->m_gtimezone);
89 auto now_str = g_date_time_format(now, "%F %T");
90- g_message("%s triggered at %s.%06d by GIOCondition %d, read %zd bytes, found %zu interrupts",
91- G_STRFUNC, now_str, g_date_time_get_microsecond(now),
92- (int)cond, n_bytes, n_interrupts);
93+ g_debug("%s triggered at %s.%06d by GIOCondition %d, read %zd bytes, found %zu interrupts",
94+ G_STRFUNC, now_str, g_date_time_get_microsecond(now),
95+ (int)cond, n_bytes, n_interrupts);
96 g_free(now_str);
97 g_date_time_unref(now);
98

Subscribers

People subscribed via source and target branches