[2674] in BarnOwl Developers
Re: [PATCH] Track names along with timers, add :show timers
daemon@ATHENA.MIT.EDU (Nelson Elhage)
Sat Sep 25 18:09:55 2010
From: Nelson Elhage <nelhage@MIT.EDU>
To: David Benjamin <davidben@mit.edu>, barnowl-dev@mit.edu
In-Reply-To: <1285394726-26853-1-git-send-email-davidben@mit.edu>
Date: Sat, 25 Sep 2010 18:09:51 -0400
+1 for making it easier to debug this stuff.
I would be tempted to use more helpful names when we can for the timers
from perl code -- i.e. instead of just a function name, which is not
super-helpful, Twitter could use "Twitter (nelhage)" or "Twitter DMs
(nelhage)".
This also solves the problem that the current use of functions for names
is confusing to someone who reads the code and thinks they are actual
function names that will somehow get called from the timer
infrastructure.
- Nelson
On Sat, 25 Sep 2010 02:05:26 -0400, David Benjamin <davidben@MIT.EDU> wrote:
> This will help people with BarnOwls eating CPU to diagnose timer leaks.
> ---
> aim.c | 5 ++-
> commands.c | 3 ++
> functions.c | 30 ++++++++++++++++++++
> owl.h | 1 +
> perl/lib/BarnOwl/Complete/Client.pm | 1 +
> perl/lib/BarnOwl/MainLoopCompatHook.pm | 1 +
> perl/lib/BarnOwl/Timer.pm | 6 +++-
> .../IRC/lib/BarnOwl/Module/IRC/Connection.pm | 1 +
> .../Jabber/lib/BarnOwl/Module/Jabber/Connection.pm | 1 +
> .../Twitter/lib/BarnOwl/Module/Twitter/Handle.pm | 2 +
> perlglue.xs | 12 ++++---
> select.c | 4 ++-
> variable.c | 3 +-
> 13 files changed, 60 insertions(+), 10 deletions(-)
>
> diff --git a/aim.c b/aim.c
> index e14b179..5e27abc 100644
> --- a/aim.c
> +++ b/aim.c
> @@ -183,7 +183,7 @@ int owl_aim_login(const char *screenname, const char *password)
> aim_request_login(sess, conn, screenname);
> owl_function_debugmsg("owl_aim_login: connecting");
>
> - g.aim_nop_timer = owl_select_add_timer(30, 30, owl_aim_send_nop, NULL, NULL);
> + g.aim_nop_timer = owl_select_add_timer("owl_aim_send_nop", 30, 30, owl_aim_send_nop, NULL, NULL);
>
> return(0);
> }
> @@ -209,7 +209,8 @@ void owl_aim_successful_login(const char *screenname)
>
> /* start the ingorelogin timer */
> owl_global_set_ignore_aimlogin(&g);
> - owl_select_add_timer(owl_global_get_aim_ignorelogin_timer(&g),
> + owl_select_add_timer("owl_aim_unset_ignorelogin",
> + owl_global_get_aim_ignorelogin_timer(&g),
> 0, owl_aim_unset_ignorelogin, NULL, NULL);
>
> /* aim_ssi_setpresence(owl_global_get_aimsess(&g), 0x00000400); */
> diff --git a/commands.c b/commands.c
> index 432d794..bc6dc63 100644
> --- a/commands.c
> +++ b/commands.c
> @@ -653,6 +653,7 @@ const owl_cmd commands_to_init[]
> "show styles\n"
> "show subscriptions / show subs\n"
> "show terminal\n"
> + "show timers\n"
> "show variables\n"
> "show variable <variable>\n"
> "show version\n"
> @@ -2242,6 +2243,8 @@ char *owl_command_show(int argc, const char *const *argv, const char *buff)
> owl_function_show_colors();
> } else if (!strcmp(argv[1], "styles")) {
> owl_function_show_styles();
> + } else if (!strcmp(argv[1], "timers")) {
> + owl_function_show_timers();
> } else if (!strcmp(argv[1], "subs") || !strcmp(argv[1], "subscriptions")) {
> owl_function_getsubs();
> } else if (!strcmp(argv[1], "terminal") || !strcmp(argv[1], "term")) {
> diff --git a/functions.c b/functions.c
> index 767fc9d..89e02ff 100644
> --- a/functions.c
> +++ b/functions.c
> @@ -92,6 +92,36 @@ void owl_function_show_styles(void) {
> owl_fmtext_cleanup(&fm);
> }
>
> +static void _owl_function_timer_append_fmtext(gpointer data, gpointer user_data) {
> + owl_fmtext *fm = user_data;
> + owl_timer *timer = data;
> + char *str = owl_sprintf("- %s: in %d seconds",
> + timer->name ? timer->name : "(unnamed)",
> + timer->time - time(NULL));
> + owl_fmtext_append_normal(fm, str);
> + owl_free(str);
> + if (timer->interval) {
> + str = owl_sprintf(", repeat every %d seconds", timer->interval);
> + owl_fmtext_append_normal(fm, str);
> + owl_free(str);
> + }
> + owl_fmtext_append_normal(fm, "\n");
> +}
> +
> +void owl_function_show_timers(void) {
> + owl_fmtext fm;
> + GList **timers;
> +
> + owl_fmtext_init_null(&fm);
> + owl_fmtext_append_bold(&fm, "Active timers:\n");
> +
> + timers = owl_global_get_timerlist(&g);
> + g_list_foreach(*timers, _owl_function_timer_append_fmtext, &fm);
> +
> + owl_function_popless_fmtext(&fm);
> + owl_fmtext_cleanup(&fm);
> +}
> +
> char *owl_function_style_describe(const char *name) {
> const char *desc;
> char *s;
> diff --git a/owl.h b/owl.h
> index 7711736..8d6ca37 100644
> --- a/owl.h
> +++ b/owl.h
> @@ -531,6 +531,7 @@ typedef struct _owl_timer {
> void (*callback)(struct _owl_timer *, void *);
> void (*destroy)(struct _owl_timer *);
> void *data;
> + char *name;
> } owl_timer;
>
> typedef struct _owl_errqueue {
> diff --git a/perl/lib/BarnOwl/Complete/Client.pm b/perl/lib/BarnOwl/Complete/Client.pm
> index eb5e630..076a41a 100644
> --- a/perl/lib/BarnOwl/Complete/Client.pm
> +++ b/perl/lib/BarnOwl/Complete/Client.pm
> @@ -36,6 +36,7 @@ my %show = (
> subscriptions => undef,
> subs => undef,
> terminal => undef,
> + timers => undef,
> variables => undef,
> variable => \&complete_variable,
> version => undef,
> diff --git a/perl/lib/BarnOwl/MainLoopCompatHook.pm b/perl/lib/BarnOwl/MainLoopCompatHook.pm
> index d42df25..35cdb2f 100644
> --- a/perl/lib/BarnOwl/MainLoopCompatHook.pm
> +++ b/perl/lib/BarnOwl/MainLoopCompatHook.pm
> @@ -48,6 +48,7 @@ sub _ensure_timer {
> weaken($weak);
> unless ($self->{timer}) {
> $self->{timer} = BarnOwl::Timer->new( {
> + name => "BarnOwl::MainLoopCompatHook",
> after => 0,
> interval => 1,
> cb => sub {
> diff --git a/perl/lib/BarnOwl/Timer.pm b/perl/lib/BarnOwl/Timer.pm
> index e121731..c7dbc46 100644
> --- a/perl/lib/BarnOwl/Timer.pm
> +++ b/perl/lib/BarnOwl/Timer.pm
> @@ -12,11 +12,15 @@ sub new {
>
> my $self = {cb => $cb};
>
> + my $name = $args->{name};
> + $name = "(unnamed)" unless defined $name;
> +
> bless($self, $class);
>
> $self->{timer} = BarnOwl::Internal::add_timer($args->{after} || 0,
> $args->{interval} || 0,
> - $self);
> + $self,
> + $name);
> return $self;
> }
>
> diff --git a/perl/modules/IRC/lib/BarnOwl/Module/IRC/Connection.pm b/perl/modules/IRC/lib/BarnOwl/Module/IRC/Connection.pm
> index e4b30e2..b864430 100644
> --- a/perl/modules/IRC/lib/BarnOwl/Module/IRC/Connection.pm
> +++ b/perl/modules/IRC/lib/BarnOwl/Module/IRC/Connection.pm
> @@ -323,6 +323,7 @@ sub schedule_reconnect {
> }
> $self->{reconnect_timer} =
> BarnOwl::Timer->new( {
> + name => "BarnOwl::Module::IRC::Connection::reconnect",
> after => $interval,
> cb => sub {
> $weak->reconnect( $interval ) if $weak;
> diff --git a/perl/modules/Jabber/lib/BarnOwl/Module/Jabber/Connection.pm b/perl/modules/Jabber/lib/BarnOwl/Module/Jabber/Connection.pm
> index 9248226..89e23a0 100644
> --- a/perl/modules/Jabber/lib/BarnOwl/Module/Jabber/Connection.pm
> +++ b/perl/modules/Jabber/lib/BarnOwl/Module/Jabber/Connection.pm
> @@ -197,6 +197,7 @@ sub onConnect {
> unless (exists $BarnOwl::Module::Jabber::vars{keepAliveTimer}) {
> $BarnOwl::Module::Jabber::vars{keepAliveTimer} =
> BarnOwl::Timer->new({
> + 'name' => 'BarnOwl::Module::Jabber::do_keep_alive_and_auto_away',
> 'after' => 5,
> 'interval' => 5,
> 'cb' => sub { BarnOwl::Module::Jabber::do_keep_alive_and_auto_away(@_) }
> diff --git a/perl/modules/Twitter/lib/BarnOwl/Module/Twitter/Handle.pm b/perl/modules/Twitter/lib/BarnOwl/Module/Twitter/Handle.pm
> index cba4d33..9ba2a92 100644
> --- a/perl/modules/Twitter/lib/BarnOwl/Module/Twitter/Handle.pm
> +++ b/perl/modules/Twitter/lib/BarnOwl/Module/Twitter/Handle.pm
> @@ -161,6 +161,7 @@ sub sleep {
>
> if($self->{cfg}->{poll_for_tweets}) {
> $self->{timer} = BarnOwl::Timer->new({
> + name => "BarnOwl::Module::Twitter::Handle::poll_twitter",
> after => $delay,
> interval => 90,
> cb => sub { $weak->poll_twitter if $weak }
> @@ -169,6 +170,7 @@ sub sleep {
>
> if($self->{cfg}->{poll_for_dms}) {
> $self->{direct_timer} = BarnOwl::Timer->new({
> + name => "BarnOwl::Module::Twitter::Handle::poll_direct",
> after => $delay,
> interval => 180,
> cb => sub { $weak->poll_direct if $weak }
> diff --git a/perlglue.xs b/perlglue.xs
> index ba9f086..12877de 100644
> --- a/perlglue.xs
> +++ b/perlglue.xs
> @@ -515,21 +515,23 @@ add_io_dispatch(fd, mode, cb)
> owl_select_add_perl_io_dispatch(fd, mode, newSVsv(cb));
>
> IV
> -add_timer(after, interval, cb)
> +add_timer(after, interval, cb, name = NULL)
> int after
> int interval
> SV *cb
> + const char *name
> PREINIT:
> SV *ref;
> owl_timer *t;
> CODE:
> ref = sv_rvweaken(newSVsv(cb));
> - t = owl_select_add_timer(after,
> + t = owl_select_add_timer(name,
> + after,
> interval,
> owl_perlconfig_perl_timer,
> owl_perlconfig_perl_timer_destroy,
> ref);
> - owl_function_debugmsg("Created timer %p", t);
> + owl_function_debugmsg("Created timer %s: %p", t->name ? t->name : "(unnamed)", t);
> RETVAL = (IV)t;
> OUTPUT:
> RETVAL
> @@ -541,8 +543,8 @@ remove_timer(timer)
> owl_timer *t;
> CODE:
> t = (owl_timer*)timer;
> - owl_function_debugmsg("Freeing timer %p", t);
> - owl_select_remove_timer(t);
> + owl_function_debugmsg("Freeing timer %s: %p", t->name ? t->name : "(unnamed)", t);
> + owl_select_remove_timer(t);
>
> MODULE = BarnOwl PACKAGE = BarnOwl::Editwin
>
> diff --git a/select.c b/select.c
> index a6f0ddf..cecc6b0 100644
> --- a/select.c
> +++ b/select.c
> @@ -12,7 +12,7 @@ int _owl_select_timer_eq(const owl_timer *t1, const owl_timer *t2) {
> return t1 == t2;
> }
>
> -owl_timer *owl_select_add_timer(int after, int interval, void (*cb)(owl_timer *, void *), void (*destroy)(owl_timer*), void *data)
> +owl_timer *owl_select_add_timer(const char* name, int after, int interval, void (*cb)(owl_timer *, void *), void (*destroy)(owl_timer*), void *data)
> {
> owl_timer *t = owl_malloc(sizeof(owl_timer));
> GList **timers = owl_global_get_timerlist(&g);
> @@ -22,6 +22,7 @@ owl_timer *owl_select_add_timer(int after, int interval, void (*cb)(owl_timer *,
> t->callback = cb;
> t->destroy = destroy;
> t->data = data;
> + t->name = name ? owl_strdup(name) : NULL;
>
> *timers = g_list_insert_sorted(*timers, t,
> (GCompareFunc)_owl_select_timer_cmp);
> @@ -36,6 +37,7 @@ void owl_select_remove_timer(owl_timer *t)
> if(t->destroy) {
> t->destroy(t);
> }
> + owl_free(t->name);
> owl_free(t);
> }
> }
> diff --git a/variable.c b/variable.c
> index 0d5cdd8..90ad3e4 100644
> --- a/variable.c
> +++ b/variable.c
> @@ -433,7 +433,8 @@ int owl_variable_pseudologins_set(owl_variable *v, const void *newval)
> if (*(const int*)newval == 1) {
> owl_function_zephyr_buddy_check(0);
> if (timer == NULL) {
> - timer = owl_select_add_timer(180, 180, owl_zephyr_buddycheck_timer, NULL, NULL);
> + timer = owl_select_add_timer("owl_zephyr_buddycheck_timer",
> + 180, 180, owl_zephyr_buddycheck_timer, NULL, NULL);
> }
> } else {
> if (timer != NULL) {
> --
> 1.7.3.1.g716de
>
>
