Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

0.12.7 causes stack overflow in as_pool_get_components #243

Closed
Vogtinator opened this issue Jul 29, 2019 · 8 comments
Closed

0.12.7 causes stack overflow in as_pool_get_components #243

Vogtinator opened this issue Jul 29, 2019 · 8 comments

Comments

@Vogtinator
Copy link
Contributor

After updating to 0.12.7, discover crashes reliably on startup.

Bisected to fa94ac0.

Appears to be a stack overflow:

#0  0x00007ffff7e69f0c in __vfprintf_internal () at /lib64/libc.so.6
#1  0x00007ffff7e7d501 in __vasprintf_internal () at /lib64/libc.so.6
#2  0x00007ffff42d3329 in g_vasprintf () at /usr/lib64/libglib-2.0.so.0
#3  0x00007ffff42f5871 in g_string_append_vprintf () at /usr/lib64/libglib-2.0.so.0
#4  0x00007ffff42f59b4 in g_string_append_printf () at /usr/lib64/libglib-2.0.so.0
#5  0x00007ffff414282c in as_xml_dump_node_children (node=0x5555707adac0) at ../src/as-xml.c:106
#6  0x00007ffff4155795 in as_component_load_from_xml (cpt=0x555570798710, ctx=0x555555b29960, node=0x5555707ad420, error=0x7fffff7ff9a0) at ../src/as-component.c:3600
#7  0x00007ffff4147bdd in as_cache_component_from_dval (cache=0x555555b22220, txn=0x555555e5b810, dval=..., error=0x7fffff7ff9a0) at ../src/as-cache.c:1487
#8  0x00007ffff4147d64 in as_cache_component_by_hash (cache=0x555555b22220, txn=0x555555e5b810, cpt_hash=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", error=0x7fffff7ff9a0) at ../src/as-cache.c:1530
#9  0x00007ffff4147e90 in as_cache_components_by_hash_list (cache=0x555555b22220, txn=0x555555e5b810, hlist=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", hlist_len=16, error=0x7fffff7ffa10) at ../src/as-cache.c:1556
#10 0x00007ffff4147fe2 in as_cache_register_addons_for_component (cache=0x555555b22220, txn=0x555555e5b810, cpt=0x5555707985a0, error=0x7fffff7ffb70) at ../src/as-cache.c:1594
#11 0x00007ffff4147c0c in as_cache_component_from_dval (cache=0x555555b22220, txn=0x555555e5b810, dval=..., error=0x7fffff7ffb70) at ../src/as-cache.c:1493
#12 0x00007ffff4147d64 in as_cache_component_by_hash (cache=0x555555b22220, txn=0x555555e5b810, cpt_hash=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", error=0x7fffff7ffb70) at ../src/as-cache.c:1530
#13 0x00007ffff4147e90 in as_cache_components_by_hash_list (cache=0x555555b22220, txn=0x555555e5b810, hlist=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", hlist_len=16, error=0x7fffff7ffbe0) at ../src/as-cache.c:1556
#14 0x00007ffff4147fe2 in as_cache_register_addons_for_component (cache=0x555555b22220, txn=0x555555e5b810, cpt=0x555570798430, error=0x7fffff7ffd40) at ../src/as-cache.c:1594
#15 0x00007ffff4147c0c in as_cache_component_from_dval (cache=0x555555b22220, txn=0x555555e5b810, dval=..., error=0x7fffff7ffd40) at ../src/as-cache.c:1493
[...]
#29956 0x00007ffff4147d64 in as_cache_component_by_hash (cache=0x555555b22220, txn=0x555555e5b810, cpt_hash=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", error=0x7fffffb4fbd0) at ../src/as-cache.c:1530
#29957 0x00007ffff4147e90 in as_cache_components_by_hash_list (cache=0x555555b22220, txn=0x555555e5b810, hlist=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", hlist_len=16, error=0x7fffffb4fc40) at ../src/as-cache.c:1556
#29958 0x00007ffff4147fe2 in as_cache_register_addons_for_component (cache=0x555555b22220, txn=0x555555e5b810, cpt=0x555565864b60, error=0x7fffffb4fda0) at ../src/as-cache.c:1594
#29959 0x00007ffff4147c0c in as_cache_component_from_dval (cache=0x555555b22220, txn=0x555555e5b810, dval=..., error=0x7fffffb4fda0) at ../src/as-cache.c:1493
#29960 0x00007ffff4147d64 in as_cache_component_by_hash (cache=0x555555b22220, txn=0x555555e5b810, cpt_hash=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", error=0x7fffffb4fda0) at ../src/as-cache.c:1530
[...]
#72260 0x00007ffff4147d64 in as_cache_component_by_hash (cache=0x555555b22220, txn=0x555555b44700, cpt_hash=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", error=0x7fffffffdcd0) at ../src/as-cache.c:1530
#72261 0x00007ffff4147e90 in as_cache_components_by_hash_list (cache=0x555555b22220, txn=0x555555b44700, hlist=0x7fffa0355dbc "Wu\201\364\246\210\332\233\\Q<\347\350\273n\032@", hlist_len=16, error=0x7fffffffdd40) at ../src/as-cache.c:1556
#72262 0x00007ffff4147fe2 in as_cache_register_addons_for_component (cache=0x555555b22220, txn=0x555555b44700, cpt=0x555556136430, error=0x7fffffffdea8) at ../src/as-cache.c:1594
#72263 0x00007ffff4147c0c in as_cache_component_from_dval (cache=0x555555b22220, txn=0x555555b44700, dval=..., error=0x7fffffffdea8) at ../src/as-cache.c:1493
#72264 0x00007ffff4148235 in as_cache_get_components_all (cache=0x555555b22220, error=0x7fffffffdea8) at ../src/as-cache.c:1660
#72265 0x00007ffff415e90f in as_pool_get_components (pool=0x555555b1f1f0) at ../src/as-pool.c:1289
#72266 0x00007ffff541ba1d in AppStream::Pool::components() const (this=0x555555b3bd30) at ../qt/pool.cpp:107
#72267 0x00007fffeb4e2988 in PackageKitBackend::reloadPackageList() () at /usr/lib64/qt5/plugins/discover/packagekit-backend.so
#72268 0x00007fffeb4e375e in PackageKitBackend::PackageKitBackend(QObject*) () at /usr/lib64/qt5/plugins/discover/packagekit-backend.so
#72269 0x00007fffeb4e38cb in  () at /usr/lib64/qt5/plugins/discover/packagekit-backend.so
#72270 0x00007ffff7cbd085 in DiscoverBackendsFactory::backendForFile(QString const&, QString const&) const () at /usr/lib64/plasma-discover/libDiscoverCommon.so
@Vogtinator Vogtinator changed the title Causes instant crash of plasma-discover 0.12.7 causes stack overflow in as_pool_get_components Jul 29, 2019
@ximion
Copy link
Owner

ximion commented Jul 29, 2019

Very strange... I can not reproduce this at all here.
Which distribution do you use? Are there any steps besides just launching Discover to reproduce the bug? Can you also reproduce the issue by running sudo appstreamcli refresh --force or appstreamcli search test --details?
It's strange that the crash happens in the XML serialization - that particular codepath is quite well tested and hasn't changed for a long time.

@Vogtinator
Copy link
Contributor Author

Very strange... I can not reproduce this at all here.
Which distribution do you use?

openSUSE Tumbleweed

Are there any steps besides just launching Discover to reproduce the bug? Can you also reproduce the issue by running sudo appstreamcli refresh --force

Worked fine (otherwise there wouldn't be a cache, I guess?)

or appstreamcli search test --details?

Worked fine as well.

It's strange that the crash happens in the XML serialization - that particular codepath is quite well tested and hasn't changed for a long time.

If there's anything I can do to help debugging, please tell.

@ximion
Copy link
Owner

ximion commented Jul 29, 2019

My attempts to reproduce this on my Debian machine have failed, so I am installing openSUSE in a VM now to look into this issue.
While investigating though I found that AppStream has a summary in openSUSE that reads "Utilities to generate, maintain and access the AppStream Xapian database". That hasn't been true for ages, Xapian was completely dropped from AppStream in 2016. Could be useful to adjust that description ;-)

@Vogtinator
Copy link
Contributor Author

While investigating though I found that AppStream has a summary in openSUSE that reads "Utilities to generate, maintain and access the AppStream Xapian database". That hasn't been true for ages, Xapian was completely dropped from AppStream in 2016. Could be useful to adjust that description ;-)

Yeah, the package summaries and descriptions are usually written once and then never adjusted again... Fix pushed to the devel project, I'll submit to Tumbleweed once a fix for the crash is available ;-)

@ximion
Copy link
Owner

ximion commented Jul 29, 2019 •
edited

Installation of openSUSE Tumbleweed looks great visually, but took 3 (!) hours using only default settings, 3GB RAM and 4 CPU cores in the VM (btrfs is default on openSUSE, apparently...).
Thank you for fixing the package description of AppStream! Now after installing its build dependencies via zypper, I noticed that it even still build-depends on libxapian...
Maybe I should ship a reference RPM specfile with the project, if that makes distribution maintenance easier?

@ximion
Copy link
Owner

ximion commented Jul 30, 2019 •
edited

So, the culprit was this:

<component type="desktop">
  <id>QXmlEdit.desktop</id>
  <pkgname>qxmledit</pkgname>
  <name>QXmlEdit</name>
  <summary>XML editor</summary>
  [...]
  <extends>QXmlEdit.desktop</extends>
  [...]
</component>

A component, that has itself as addon. This is a validator issue already, but I think I will need to make this an even harder validation error, because this is clearly wrong and not intended. At the same time, this issue shouldn't send libappstream into an infinite recursion, so I fixed that in the patch below.
Thank you for finding the issue and writing a good bug report!

P.S: Looking at https://build.opensuse.org/package/view_file/openSUSE:Factory/AppStream/AppStream.spec?expand=1 , aside from changing the description to not mention Xapian (Tools and libraries to work with AppStream metadata may work better as short desc, or just look at the Debian description here) you can also drop a bunch of build dependencies: intltool, packagekit-glib2, protobuf and xapian-core are definitely not needed ;-)
Also, if you want improved search, having Snowball/libstemmer is a good idea: https://snowballstem.org/ (this feature will remain optional though)

@ximion ximion closed this as completed in e01b6cf Jul 30, 2019
@Vogtinator Vogtinator mentioned this issue Jul 30, 2019
Closed
@Vogtinator
Copy link
Contributor Author

Vogtinator commented Jul 30, 2019 •
edited

Installation of openSUSE Tumbleweed looks great visually, but took 3 (!) hours using only default settings, 3GB RAM and 4 CPU cores in the VM (btrfs is default on openSUSE, apparently...).

That sounds like something is really wrong with your setup - openQA uses VMs with a single CPU core and 1.5GiB of RAM and a default install + KDE and the installation from starting QEMU, entering settings and taking some detours for extracting logs takes 20 minutes: https://openqa.opensuse.org/tests/994165#step/await_install/1

Thank you for fixing the package description of AppStream! Now after installing its build dependencies via zypper, I noticed that it even still build-depends on libxapian...

Yup, I'll do some needed spec file cleanup...

Maybe I should ship a reference RPM specfile with the project, if that makes distribution maintenance easier?

In most cases the .spec taken from upstream can't be used as-is as each distro has different package names and packaging policies, so such .spec files are mostly just used for the initial packaging task.

A component, that has itself as addon. This is a validator issue already, but I think I will need to make this an even harder validation error, because this is clearly wrong and not intended.

Ouch. I filed an issue: lbellonda/qxmledit#63

At the same time, this issue shouldn't send libappstream into an infinite recursion, so I fixed that in the patch below.
Thank you for finding the issue and writing a good bug report!

Thanks for the quick response and fix! I can confirm that it works fine with git master now.

@ximion
Copy link
Owner

ximion commented Aug 14, 2019

I added a few more changes to protect against all cases of circular references (775e7e5) and extended the validator to actually find these obvious cases and complain about them (6ed447d). This should help developers to find those mistakes easily.

If everything goes well, there will be a new release this week containing all the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ximion @Vogtinator