> Webrev:
> http://cr.opensolaris.org/~bpytlik/ips-5171-v1/

The part of the code you changed looks fine.  At first I forgot that
you'd added a call to schedule to check_for_updates and wondered whether
it would make more sense to elide the else in do_next_check.  Since you
have the call to schedule_next_check in check_for_updates, I think that
it's fine the way that it is.

On line 329 and 330, it looks like we're effectively setting the maximum
amount of time between checks to 1 day.  If that's the case, we should
remove the obsolete checks in is_check_required that start around line
192.

When you were investigating this bug, you observed that every time we
added an event to the timeout queue, we grew the size of the memory of
the process.  The fact that we were accumulating unserviced callbacks
made it look like a memory leak had occurred.

With this in mind, I wonder if it makes sense to either limit the number
of do_next_check and check_for_update callbacks that we'll allow to be
enqueued.  Another possibility would be to switch to a dedicated thread
that parks itself in cv_timedwait until we must check for an update.
These approaches might prevent us from encountering a similar bug in the
future.

-j

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to