update-manager bug when loading Changes is slow

Bug #133139 reported by Bogdan Butnaru
14
Affects Status Importance Assigned to Milestone
update-manager (Ubuntu)
Fix Released
Low
Robert Roth

Bug Description

Binary package hint: update-manager

Hello. This is about the latest (and older) update-manager on Gutsy.

I have the update-manager window opened with the "Changes" tab selected and expanded, so it shows the changes for every package to be updated.

Imagine there are three packages in the updates list, A, B and C. If A is selected, its changes are displayed correctly below it. If I press down arrow on the keyboard, the B package is selected, its changes should be looked-up and displayed. The same for C.

This works OK normally, but:

(We're back with A selected.) I click down, B gets selected. The Changes tab briefly displays "Downloading the list of changes" or something similar. This is the trick: If I press down again _before_ the changes for B are displayed (this happens often for me, but it might depend on your connection), then the bug happens.

Usually the changes for C are displayed (not sure why it's quicker than B), then I get a warning in the console, then the changes for B are displayed, although C is still selected. I can get the changes for C by going back to B and then returning to C.

Here's an example for this happening three times in a row:
/usr/lib/python2.5/site-packages/UpdateManager/UpdateManager.py:584: Warning: /build/buildd/glib2.0-2.13.7/gobject/gsignal.c:1741: instance `0x875ece0' has no handler with id `362'
  button.disconnect(id);
/usr/lib/python2.5/site-packages/UpdateManager/UpdateManager.py:584: Warning: /build/buildd/glib2.0-2.13.7/gobject/gsignal.c:1741: instance `0x875ece0' has no handler with id `367'
  button.disconnect(id);
/usr/lib/python2.5/site-packages/UpdateManager/UpdateManager.py:584: Warning: /build/buildd/glib2.0-2.13.7/gobject/gsignal.c:1741: instance `0x875eb70' has no handler with id `377'
  button.disconnect(id);

I suppose a callback waiting for the download of the changes forgets to check if it should still display them.

Related branches

Revision history for this message
Michael Vogt (mvo) wrote :

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.

Changed in update-manager:
status: New → Confirmed
Revision history for this message
Michael Nelson (michael.nelson) wrote :

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

Changed in update-manager:
assignee: nobody → absoludity
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (3.5 KiB)

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...

Read more...

Revision history for this message
lavinog (lavinog) wrote :

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.

Revision history for this message
lavinog (lavinog) wrote :

I cannot recreate the issue in lucid.

Revision history for this message
Robert Roth (evfool) wrote :

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

Changed in update-manager (Ubuntu):
status: Confirmed → Incomplete
assignee: Michael Nelson (michael.nelson) → nobody
Revision history for this message
Robert Roth (evfool) wrote :

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

Changed in update-manager (Ubuntu):
status: Incomplete → Triaged
importance: Undecided → Low
Robert Roth (evfool)
Changed in update-manager (Ubuntu):
assignee: nobody → Robert Roth (evfool)
status: Triaged → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in update-manager (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers