GNOME Bugzilla – Bug 745861
RemoteSearch: don't complete a search that was cancelled
Last modified: 2015-03-11 20:48:37 UTC
This closes a race between setTerms and a slow GetInitialResultSet. The bug manifests as follows: - initial search for a short string - previous results === undefined, call GetInitialResultSet - user types more, cancel previous search in setTerms() - mainloop, then _gotResults([]) - previous results === [], !!previous results === true - therefore call GetSubsearchResultSet with an empty list of results - _gotResults() from GetSubsearchResultSet is empty - much later, return from GetInitialResultSet is discarded by cancellable - user unhappy because what he searched for is not there After this fix, the flow is: - initial search for a short string - previous results === undefined, call GetInitialResultSet - user types more, cancel previous search in setTerms() - mainloop, but no _gotResults - previous results === undefined, call GetInitialResultSet again with longer string - some time later, return from first GetInitialResultSet is discarded by cancellable - soon after, return from second GetInitialResultSet comes with good results - user happy
Created attachment 298840 [details] [review] RemoteSearch: don't complete a search that was cancelled
Created attachment 298844 [details] [review] Search: be resilient against buggy search providers If a search provider returns a meta without a name, don't crash constructing the actor.
Review of attachment 298844 [details] [review]: fair enough
Review of attachment 298840 [details] [review]: The code looks fine and makes sense. But your description confuses me: ... - mainloop, then _gotResults([]) ... - much later, return from GetInitialResultSet is discarded by cancellable .. Isn't the _gotResults([]) step the result of the cancellable being cancelled? Why do you say that much later the GetInitialResultSet return is discarded? Didn't that happen already when _gotResults([]) ?
(In reply to Rui Matos from comment #4) > Review of attachment 298840 [details] [review] [review]: > > The code looks fine and makes sense. > > But your description confuses me: > > ... > - mainloop, then _gotResults([]) > ... > - much later, return from GetInitialResultSet is discarded by > cancellable > .. > > Isn't the _gotResults([]) step the result of the cancellable being > cancelled? Why do you say that much later the GetInitialResultSet return is > discarded? Didn't that happen already when _gotResults([]) ? If you cancel a cancellable associated with the dbus call, you don't actually cancel the call remotely. _gotResults([]) is called at the next mainloop iteration, but some time later you will still get a method_reply dbus message, and that will be discarded by the gdbus code (because the associated call was cancelled)
Attachment 298840 [details] pushed as b0be6b8 - RemoteSearch: don't complete a search that was cancelled Attachment 298844 [details] pushed as 8b5a44e - Search: be resilient against buggy search providers