Installed, popular and featured snaps are loaded in parallel threads can crash

Bug #1716633 reported by errors.ubuntu.com bug bridge on 2017-09-12
94
This bug affects 10 people
Affects Status Importance Assigned to Milestone
gnome-software (Ubuntu)
High
Robert Ancell
Xenial
High
Robert Ancell
Zesty
High
Robert Ancell
Artful
High
Robert Ancell
Bionic
High
Robert Ancell

Bug Description

[Impact]
GNOME Software has a cache that can be accessed by multiple threads. This can cause GNOME Software to crash. This seems to have been caused by adding support for featured snaps (bug 1663097) as the popular, featured and installed snaps are all requested on different threads.

[Test Case]
Since this is a random crash there's not a good test case. Could be checked by looking at errors.ubuntu.com crash reports or repeated restarts of GNOME Software to see if it crashes on startup.

[Regression Potential]
Solution is to add a lock around this cache. Some risk of breaking related code.

description: updated
Changed in gnome-software (Ubuntu):
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Robert Ancell (robert-ancell)
Robert Ancell (robert-ancell) wrote :

The crash is in new code that was added to the Xenial SRU, not a regression in existing code.

The stacktrace doesn't point to an obvious issue that is causing this. It's possible that the hash table has been corrupted by other code gone wrong. I don't think this is worth blocking the SRU on.

Sebastien Bacher (seb128) wrote :

Some reports are similar but with gs_plugin_add_featured in the backtrace instead of gs_plugin_add_popular

tags: added: gnome-software-snap
Changed in gnome-software (Ubuntu Artful):
importance: Undecided → High
Changed in gnome-software (Ubuntu Zesty):
importance: Undecided → High
Changed in gnome-software (Ubuntu Xenial):
importance: Undecided → High
Changed in gnome-software (Ubuntu Artful):
status: New → Triaged
Changed in gnome-software (Ubuntu Zesty):
status: New → Triaged
Changed in gnome-software (Ubuntu Xenial):
status: New → Triaged
Changed in gnome-software (Ubuntu Bionic):
status: Confirmed → Triaged
Changed in gnome-software (Ubuntu Artful):
assignee: nobody → Robert Ancell (robert-ancell)
Changed in gnome-software (Ubuntu Zesty):
assignee: nobody → Robert Ancell (robert-ancell)
Changed in gnome-software (Ubuntu Xenial):
assignee: nobody → Robert Ancell (robert-ancell)
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-software - 3.26.1-0ubuntu4

---------------
gnome-software (3.26.1-0ubuntu4) bionic; urgency=medium

  * debian/patches/0001-snap-Protect-store-cache-with-a-lock.patch:
    - Fix crash due to multiple threads accessing a cache (LP: #1716633)
  * debian/patches/0001-snap-Don-t-attempt-to-refine-snaps-without-a-valid-n.patch:
    - Fix potential crash from invalid snaps
  * debian/patches/0001-snap-Remove-unused-variables.patch:
    - Trivial patch added to make above patches apply cleanly

 -- Robert Ancell <email address hidden> Fri, 27 Oct 2017 11:34:43 +1300

Changed in gnome-software (Ubuntu Bionic):
status: Triaged → Fix Released
Robert Ancell (robert-ancell) wrote :

I thought that each plugin was only accessed by the same thread, but it turns out this is not the case. This crash seems to be due to these multiple threads accessing the snap cache. This is now protected with a lock.

description: updated
summary: - /usr/bin/gnome-
- software:11:g_str_equal:g_hash_table_lookup_node:g_hash_table_insert_internal:find_snaps:gs_plugin_add_popular
+ Installed, popular and featured snaps are loaded in parallel threads can
+ crash
Sebastien Bacher (seb128) wrote :

Robert, when do you plan to SRU to 17.10?

Robert Ancell (robert-ancell) wrote :

I'm awaiting QA confirmation of the proposed release in ppa:ubuntu-desktop/gnome-software and the packaging branches.

Hello errors.ubuntu.com, or anyone else affected,

Accepted gnome-software into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gnome-software/3.26.1-0ubuntu2.17.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in gnome-software (Ubuntu Artful):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-artful
Jean-Baptiste Lallement (jibel) wrote :

SRU Verification

From errors.u.c [1] new crashes specific to version 3.26.1-0ubuntu2.17.10.1 all but one failed to retrace and could not be bucketed. Hence it is not possible to take a decision based on these new crash reports. The crashes that failed to retrace are:
https://errors.ubuntu.com/problem/169c8b88552a2231004318e45da0c61d14238d17
https://errors.ubuntu.com/problem/897b37ad31b25c3c042535a5f2fcb051a38c5e2a
https://errors.ubuntu.com/problem/e6a6f59139de19a51e1d9b6a546c4222cf0a8d5d
https://errors.ubuntu.com/problem/34980d512593c1800342e6bf5cb60ac988be5902
https://errors.ubuntu.com/problem/a3d20a0452b8d20985a14a66046b66dd94549055

The following crash report is new with the version in artful-proposed and has been reported as bug 1734630
https://errors.ubuntu.com/problem/627675f12f5f22a3d6b6ee73271c007681d1d0db

@Robert, could you have a look and tell if it has been introduced by the new code in this SRU and could you give your opinion on the crashes that failed to retrace?

Thanks.

[1] https://errors.ubuntu.com/?release=Ubuntu%2017.10&package=gnome-software&period=week&version=3.26.1-0ubuntu2.17.10.1

Robert Ancell (robert-ancell) wrote :

The crashes that failed to retrace all seem to have the following stacktrace:
?? ()
g_io_channel_shutdown () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
?? () from /usr/lib/x86_64-linux-gnu/libpackagekit-glib2.so.18
g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0

Which suggests they are a bug in PackageKit. A quick look at the PackageKit source shows this is done in a callback that may still be running after an object has been unreferenced. So I don't think they're related to the Snap changes.

The crash with the backtrace seems to be related to the network monitor changing state (network disconnected/connected?). So I don't think it's related to the Snap plugin.

Marking as verification-done per Robert's comment. Also I cannot reproduce the crash and there is no other occurrence of it.

tags: added: verification-done verification-done-artful
removed: verification-needed verification-needed-artful
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-software - 3.26.1-0ubuntu2.17.10.1

---------------
gnome-software (3.26.1-0ubuntu2.17.10.1) artful; urgency=medium

  [ Robert Ancell ]
  * debian/patches/0001-snap-Don-t-crash-on-unknown-snap-types.patch:
    - Fix crash when broken snaps installed (LP: #1720442)
  * debian/patches/0001-snap-Protect-store-cache-with-a-lock.patch:
    - Fix crash due to multiple threads accessing a cache (LP: #1716633)
  * debian/patches/0001-snap-Don-t-attempt-to-refine-snaps-without-a-valid-n.patch:
    - Fix potential crash from invalid snaps
  * debian/patches/0001-snap-Remove-unused-variables.patch:
    - Trivial patch added to make above patches apply cleanly
  * debian/patches/0001-Add-a-cancellable-to-GsApp-objects.patch:
  * debian/patches/0001-Allow-to-cancel-app-ops-in-the-details-view-that-wer.patch:
    - Fix cancelling with multiple installs (LP: #1722183)
  * debian/control:
    - Add Vcs-Bzr link

  [ Jeremy Bicha ]
  * Cherry-pick 0001-Don-t-error-out-for-over-500-results.patch:
    - Fix empty Addons > Shell Extensions category (LP: #1722809)

 -- Robert Ancell <email address hidden> Wed, 15 Nov 2017 11:58:57 +1300

Changed in gnome-software (Ubuntu Artful):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for gnome-software has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Hello errors.ubuntu.com, or anyone else affected,

Accepted gnome-software into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gnome-software/3.22.7-0ubuntu3.17.04.8 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-zesty to verification-done-zesty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-zesty. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in gnome-software (Ubuntu Zesty):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-zesty
removed: verification-done
Brian Murray (brian-murray) wrote :

Hello errors.ubuntu.com, or anyone else affected,

Accepted gnome-software into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/gnome-software/3.20.5-0ubuntu0.16.04.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in gnome-software (Ubuntu Xenial):
status: Triaged → Fix Committed
tags: added: verification-needed-xenial

SRU verification for xenial and zesty.

After 10 days in proposed there are no crash specific to the versions of gnome-software in -proposed for xenial and zesty reported to errors.u.c.

Marking as verification-done

tags: added: verification-done verification-done-xenial verification-done-zesty
removed: verification-needed verification-needed-xenial verification-needed-zesty
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-software - 3.22.7-0ubuntu3.17.04.8

---------------
gnome-software (3.22.7-0ubuntu3.17.04.8) zesty; urgency=medium

  * debian/patches/0001-snap-Protect-store-cache-with-a-lock.patch:
    - Fix crash due to multiple threads accessing a cache (LP: #1716633)
  * debian/patches/0001-snap-Don-t-attempt-to-refine-snaps-without-a-valid-n.patch:
    - Fix potential crash from invalid snaps
  * debian/patches/0001-snap-Fix-leak-of-GsApp.patch:
    - Fix small memory leak
  * debian/patches/0001-snap-Only-show-snaps-as-sandboxed-if-snapd-supports-.patch:
    - Simple patch added to make above patches apply cleanly
  * debian/patches/0001-snap-Cancel-snapd-transaction-when-install-remove-ca.patch:
  * debian/patches/0001-snap-Generate-correct-error-for-cancelled-operations.patch:
    - Abort change in snapd when install/remove cancelled (LP: #1721762)
  * debian/patches/0022-snap-Use-new-Polkit-API.patch:
    - Send Polkit interactive header (LP: #1736232)
  * debian/patches/0023-snap-Support-new-snapd-license-field.patch
    - Show license field (not yet supported by Ubuntu Store / snaps)
  * debian/patches/0024-Use-the-developer-name-if-set-in-preference-to-the-p.patch:
  * debian/patches/0025-snap-Use-developer-field.patch
    - Show snap developer (LP: #1715054)
  * debian/patches/0026-snap-Correct-the-app-origin.patch:
    - Set correct store name (LP: #1706500)

 -- Robert Ancell <email address hidden> Wed, 06 Dec 2017 10:37:25 +1300

Changed in gnome-software (Ubuntu Zesty):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gnome-software - 3.20.5-0ubuntu0.16.04.7

---------------
gnome-software (3.20.5-0ubuntu0.16.04.7) xenial; urgency=medium

  * debian/patches/0016-Add-a-dpkg-plugin.patch:
  * debian/patches/0017-Add-an-APT-plugin.patch:
    - Fix overflow showing installed size (LP: #1713586)
  * debian/patches/0047-Use-the-developer-name-if-set-in-preference-to-the-p.patch:
    - Backport support for showing developer name
  * debian/patches/0018-Add-a-Snap-plugin.patch:
    - Show snap developer (LP: #1715054)
    - Set correct store name (LP: #1706500)
    - Show license field
    - Send Polkit interactive header (LP: #1736232)
    - Fix crash due to multiple threads accessing a cache (LP: #1716633)
  * debian/patches/0001-ubuntu-reviews-Fix-Wilson-score-calculation.patch:
    - Fix average rating calculation (LP: #1721419)

 -- Robert Ancell <email address hidden> Wed, 06 Dec 2017 10:36:16 +1300

Changed in gnome-software (Ubuntu Xenial):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers