After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 618958 - Pressing search in youtube search >1 times quickly crashes totem
Pressing search in youtube search >1 times quickly crashes totem
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: YouTube plugin
2.30.x
Other Linux
: Normal normal
: ---
Assigned To: General Totem maintainer(s)
General Totem maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2010-05-18 05:30 UTC by Ben
Modified: 2010-06-26 12:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backtrace (13.23 KB, text/plain)
2010-05-18 05:30 UTC, Ben
Details

Description Ben 2010-05-18 05:30:35 UTC
Created attachment 161314 [details]
backtrace

steps to reproduce:
1. open totem and the sidebar
2. in sidebar go to youtube
3. enter in something to the search
4. double click on search
5. totem crashes

This is with totem 2.30.1 on Ubuntu 10.04
Comment 1 Ben 2010-05-18 07:26:35 UTC
I am forwarding this downstream bug which also includes an strace that was too large to attach here. https://bugzilla.gnome.org/show_bug.cgi?id=618958
Comment 3 Philip Withnall 2010-05-18 16:07:49 UTC
This is happening because starting a second search immediately unrefs and replaces the global GCancellable with one for the second search. However, the async cancellation code for the first search will assume that the global GCancellable still belongs to it, and will merrily free the new GCancellable. This upsets the second search.

The simplest solution I can see to this is to just disallow double-clicking of the Search button (i.e. desensitise it until a search is complete, or cancellation due to clicking the Cancel button is complete). Any other solution is going to run into race conditions around setting the progress bar's state at the very least, even if it solves the GCancellable problems.

Bastien? Is it acceptable in the UI to desensitise the Search button while we're searching?
Comment 4 Bastien Nocera 2010-05-19 11:58:26 UTC
Seems fine by me, though I can't help feeling that we should be able to avoid that problem by leaving the cancellable dangling and letting the cancel thread handle the cleaning up.
Comment 5 Philip Withnall 2010-05-19 12:16:21 UTC
(In reply to comment #4)
> Seems fine by me, though I can't help feeling that we should be able to avoid
> that problem by leaving the cancellable dangling and letting the cancel thread
> handle the cleaning up.

I'm fairly sure that if we left the cancellable dangling, the cancelled thread would mess around with the progress bar while the new search thread was also fiddling with it.

I'll come up with a patch sometime soon. (Exams! Argh!)
Comment 6 Philip Withnall 2010-05-19 21:13:47 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Seems fine by me, though I can't help feeling that we should be able to avoid
> > that problem by leaving the cancellable dangling and letting the cancel thread
> > handle the cleaning up.
> 
> I'm fairly sure that if we left the cancellable dangling, the cancelled thread
> would mess around with the progress bar while the new search thread was also
> fiddling with it.
> 
> I'll come up with a patch sometime soon. (Exams! Argh!)

On second thoughts, just making the Search button insensitive isn't a good solution, since that would require it to be desensitised even we're loading more results, which would be completely counterintuitive in the UI.

Once my exams are over, I'll take a proper look at a nice solution involving dangling cancellables and better thread safety. Probably shouldn't spend this amount of time on it before my exams, though. (Exams finish on the 9th, so poke me if I haven't done anything about it by a few days afterwards.)
Comment 7 Philip Withnall 2010-06-26 12:46:00 UTC
commit a39491154701e801f3060abc0cab2e24df4c5a65
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Sat Jun 26 13:41:39 2010 +0100

    Bug 618958 — Pressing search in youtube search >1 times quickly crashes totem
    
    Refactor the cancellable handling in the YouTube plugin so that it doesn't
    use a single global cancellable instance (which didn't actually work anyway).
    Closes: bgo#618958

 src/plugins/youtube/totem-youtube.c |  205 +++++++++++++++++++++++------------
 1 files changed, 133 insertions(+), 72 deletions(-)