GNOME Bugzilla – Bug 771338
rhythmbox crashed with SIGSEGV in get_url_cb ( grl-net-wc.c )
Last modified: 2018-09-24 09:48:38 UTC
(gdb) bt
+ Trace 236661
Steps to reproduce: 1. Open Rhythmbox. 2. Click on 'Jamendo' and then on 'Radio France' in side pane. 3. Repeat [2] 4. Rhythmbox crashes.
A kind and honest request to grilo developers: Please do a minimal check of libgrilo changes against rhythmbox, like the following: 1. Click the grilo sources in the side pane a few times at random. 2. Try playing a couple of tracks from Jamendo / Radio france. 3. Enable / Disable the grilo plugin a few times ( optional ) This will save time and effort on both rhythmbox and grilo side. Thanks for understanding.
(gdb) p c $10 = (struct request_clos *) 0x1408530 (gdb) p *c $11 = { self = 0x140a220 [GrlNetWc], url = 0x23a7ef0 "http://www.franceinfo.fr/player", result = 0x1c261f0, cancellable = 0x7fa9e8013d40 [GCancellable], headers = 0x0, source_id = 3385 } (gdb) p d $12 = (struct request_clos *) 0x2ab9210 (gdb) p *d $13 = { self = 0x140a220 [GrlNetWc], url = 0x2c4a370 "http://www.franceinter.fr/player", result = 0x29d10a0, cancellable = 0x7fa9e8013d40 [GCancellable], headers = 0x0, source_id = 3384 } (gdb)
Issue reproducible with grilo-test-ui-0.3. Steps to reproduce: 1. Start grilo-test-ui-0.3 2. Double click "Radio France" as soon as the window loads. 3. grilo-test-ui-0.3 crashes.
Created attachment 335481 [details] Debug logs ( export GRL_DEBUG=*:* )
Likely, my fault
Created attachment 335653 [details] [review] net: Check for throttling only if set Otherwise, we might add several 0 seconds delay on multiple fetch requests leading to unexpected behavior (assertion)
Created attachment 335654 [details] [review] tests: add stress test to grl-net-wc
Created attachment 335655 [details] [review] tests: avoid critical warning on test As timeout might not be used.
*** Bug 771497 has been marked as a duplicate of this bug. ***
Not seeing the crash after applying the patch. Thanks Victor ! Can you please add "lib-net" to the test suite, so that it runs with "make check".
Review of attachment 335653 [details] [review]: Add a comment above that condition, otherwise it looks like it's reversed.
Review of attachment 335654 [details] [review]: Sure. Please add some comments about what each section is doing (setting up server, launching client tests).
Review of attachment 335655 [details] [review]: ::: tests/lib-net.c @@ +119,2 @@ if (f->num_operations == 0) { + if (f->timeout) f->timeout > 0
*** Bug 771622 has been marked as a duplicate of this bug. ***
Created attachment 335860 [details] [review] net: Check for throttling only if set Otherwise, we might add several 0 seconds delay on multiple fetch requests leading to unexpected behavior (assertion)
Created attachment 335861 [details] [review] tests: add stress test to grl-net-wc
Created attachment 335862 [details] [review] tests: avoid critical warning on test As timeout might not be used.
(In reply to vrishab from comment #11) > Not seeing the crash after applying the patch. Thanks Victor ! > > Can you please add "lib-net" to the test suite, so that it runs with "make > check". Soon. I found a small issue with small delay test, I'll fix it and submit a patch to include it on test suite too. Thanks for testing.
Created attachment 335866 [details] [review] tests: don't always consider the threshold The threshold should not be considered when requested delay is 0, otherwise the expected_time could be before the requested_time.
Created attachment 335867 [details] [review] tests: include lib-net into make check
Review of attachment 335860 [details] [review]: Looks fine.
Review of attachment 335861 [details] [review]: Sure.
Review of attachment 335862 [details] [review]: Yep.
Review of attachment 335866 [details] [review]: > don't always consider the threshold > > The threshold should not be considered when requested delay is 0, > otherwise the expected_time could be before the requested_time. I don't understand what this is trying to fix. ::: tests/lib-net.c @@ +139,2 @@ /* Some THRESOLD seems necessary as the test time happens before the actual * GrlNetWc computation. This test is more about throttling working then "than precision"
Review of attachment 335867 [details] [review]: Sure.
Attachment 335860 [details] pushed as ca91b28 - net: Check for throttling only if set Attachment 335861 [details] pushed as e97d89d - tests: add stress test to grl-net-wc Attachment 335862 [details] pushed as 41f66e9 - tests: avoid critical warning on test Attachment 335867 [details] pushed as 6ffe445 - tests: include lib-net into make check
*** Bug 773329 has been marked as a duplicate of this bug. ***
Just for the record, I can very easily reproduce this crash too in the GNOME Games flatpak from the stable gnome-3-22 branch (still using grilo 0.3.2), just by having a few (10-20) ROMS to load covers from: it crashes in no time while trying to fetch the data. Fortunately, this seems fixed in the nightly channel due to the fix here (thanks!), meaning that we can cherry pick that to build the flatpak bundle for Games in the stable repo, but it would be even better if there was a stable release of grilo including it, that could be used without patching. I checked the dates the latest releases of grilo happened and couldn't find a clear pattern to predict when the next one would happen. Any idea?
(In reply to Mario Sánchez Prada from comment #29) > I checked the dates the latest releases of grilo happened and couldn't find > a clear pattern to predict when the next one would happen. Any idea? Yep. Time for a new release, I'll roll one tomorrow. Thanks for the ping :)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/101.