update-manager bug when loading Changes is slow

The problem is in:
UpdateManager/UpdateManager.py:
def on_treeview_update_cursor_changed(self, widget):
...
id = button.connect("clicked",
                            lambda w,lock: lock.release(), lock)
        # wait for the dl-thread
        while lock.locked():
          time.sleep(0.05)
          while gtk.events_pending():
            gtk.main_iteration()
...

it looks like the main_iteration() can cause re-entrace of this function and that confuses the bottom bits.

Keen to give this a go (and learn a bit about contributing to Ubuntu while I'm at it) :)

I've been looking at this bug a little bit, and need a bit of feedback... if any mentors have time :)

The problem is pretty straight forward, but the solution isn't (to me anyway!). As noted above, the problem is that the two calls to on_treeview_update_cursor_changed get stacked (rather than using callbacks), the second (more recent) call has to finish before the first call gets to check whether it's own lock has been released (a separate lock is created each time the function is called). Once the second call completes, the first call then gets a chance to check it's lock, finds it released and updates the UI, leaving the UI in an inconsistent state (change log displayed for previously selected item rather than current.)

Now as far as I can tell, the locks aren't being used for any thread-related concurrency issues (as I said, a separate lock is created for each new thread), but rather to wait until the thread finishes so that the calling process can update the UI.

So, the two solutions that I can see are:

1) Pass a callback function to the newly created thread and allow the creating process to finish (rather than waiting for the lock to be released). When the callback is called (with the package name as parameter), it checks to ensure that the package name being updated is actually the most recently requested package before it updates the UI. Eg:

def cache_updated_callback(package_name):
    if package_name == self.last_requested_package_name:
        # Update UI

This no longer requires the locks (unless I missed something!) but does require a new attribute (last_requested_package_name - could be wrapped up in a new changes_log object that manages its own states?) to ensure that the UI is only ever updated with the most recently requested package change log. Also means that cancelled requests can still update the cache with their data, even though they're not yet used in the UI (handy if user then clicks back on previously selected package).

On the downside, I'm guessing (from some of the comments in the code) the reason a callback wasn't used in the first place is because it leaves the newly created thread updating the UI, rather than the main process. But i don't think it would be difficult to do this in a thread-safe way as outlined above?

2) A second solution could be to continue using the locks/wait, but assign the lock to an attribute so that it can be used to cancel any previous requests. That is, whenever the function wants to create a new thread, it checks to see if there has been any previous requests (ie. there is a lock), and if so, releases the previous lock so the previous request-thread will not update the cache (seems a bit inefficient, as the cache may as well be updated, even if the data isn't used *yet*, but that's the way the code is atm), then creates and assigns a new lock to the class attribute, continuing on.

That way, when the second request completes, the UI is updated (and the call is popped off the stack). The first request will then check it's lock, find it released, query the cache but find the cache doesn't contain it's required value and so skip updating the UI (as in the current implementation).

Le...

This seems to be still an issue in jaunty.
I scrolled down the update list using the down key, and when it got to the bottom, the update panel was displaying change logs for every package.
Also it gets stuck on kernel packages for a second, but causes 100% cpu on one core. (most likely because of the size of the kernel changelogs)
This issue doesn't care if the description panel is hidden either.

I would recommend not loading the changelog, or description unless the description panel is displayed.

I cannot recreate the issue in lucid.

Thank you for reporting this bug to Ubuntu. Feisty reached EOL in October, 2008.
Please see this document for currently supported Ubuntu releases:
https://wiki.ubuntu.com/Releases

I've tried recreating this bug with Natty and was unable to, given the information you've provided. Please either a) upgrade and test or b) increase the verbosity of the steps to recreate it so we can try again.

Please feel free to report any other bugs you may find.
---
Ubuntu Bug Squad volunteer triager
http://wiki.ubuntu.com/BugSquad

I have managed to reproduce this on Oneiric, setting to Triaged.

This bug was fixed in the package update-manager - 1:0.152.18

---------------
update-manager (1:0.152.18) oneiric; urgency=low

  [ Michael Vogt ]
  * debian/pycompat:
    - removed, no longer needed
  * debian/rules:
    - fix incorrect invocation of --with=python2

  [ Robert Roth ]
  * lp:~evfool/update-manager/handlewarning:
    - Only disconnect handler if it's still connected (LP: #133139)
  * lp:~evfool/update-manager/glibchangefix:
    - Call glib.markup_escape_text() correctly as per current
      gir (don't pass in string length; LP: #832745)
  * lp:~evfool/update-manager/fixcopylink:
    - Fix the "Copy web link" context menu item of the ChangeLog viewer.
      It did not work before of some Gtk changes, now it does work.
      Fixes LP: #831944.

  [ Stefano Rivera ]
  * extras is another special case where validTo=False (LP: #775694)
 -- Michael Vogt <email address hidden> Tue, 13 Sep 2011 11:47:10 +0200