[Sugar-devel] [PATCH] Add cpu and memory resource indicator to frame

[James Cameron]

On Fri, Jul 02, 2010 at 08:47:28PM +0530, anishmangal2002 wrote:
> +    def __timer_cb(self):
> +        # Get free CPU resources [...]
> +        # Get free memory resources [...]
> +        # Update CPU and Memory labels and progressbars [...]
> +
> +        # Keep invoking this method if we are popped up
> +        return self._popped_up
> +
> +    def _popup_cb(self, gobject_ref):
> +        gobject.timeout_add(1000, self.__timer_cb)
> +        self._popped_up = True
> +
> +    def _popdown_cb(self, gobject_ref):
> +        # Kill timer if we are not popped up
> +        self._popped_up = False

I'm a bit worried about this.

0.  you have a potential race that may occur if execution is stalled by
system load between timeout_add and _popped_up = True, which would lead
to __timer_cb being called only once, ... mostly harmless, but untidy,

1.  the timeout is not removed until it is called, and a new timeout is
added on every popup ... which means that if I press the frame key many
times in one second there could be many timeouts scheduled and executed,
and while this is also mostly harmless it is unnecessary,

2.  the information on the frame is still old when popup occurs, and is
not updated until one second later.

Instead, I recommend this:

a.  save the timeout id returned by the timeout_add call, and use it in
the _popdown_cb to gobject.source_remove the timer, rather than use a
flag _popped_up,

b.  call __timer_cb once from _popup_cb, before timeout_add,

Thus:

def _popup_cb(self, gobject_ref):
    self.__timer_cb()
    self._timer = gobject.timeout_add(1000, self.__timer_cb)

def _popdown_cb(self, gobject_ref):
    gobject.source_remove(self._timer)

(and delete self._popped_up everywhere)

-- 
James Cameron
http://quozl.linux.org.au/