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 701336 - Magnatune: slow startup due to hostname lookup at plugin init
Magnatune: slow startup due to hostname lookup at plugin init
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: plugins
unspecified
Other Linux
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
: 706615 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-05-31 09:21 UTC by Kalev Lember
Modified: 2013-08-23 15:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Removes the internet check (2.07 KB, patch)
2013-06-05 03:56 UTC, Victor Toso
committed Details | Review

Description Kalev Lember 2013-05-31 09:21:05 UTC
The new magnatune plugin does a sync g_network_monitor_can_reach() at plugin startup, which can block for an extended amount of time when doing DNS lookup.

Would it be possible to move the DNS lookup out of plugin init? Network can fail (or reappear) at any time, so having the check in plugin init doesn't really ensure all following network operation are going to succeed.

There's a downstream bug report about this issue at https://bugzilla.redhat.com/show_bug.cgi?id=969123 where the reporter is experiencing a 30s freeze at Totem startup because of this.
Comment 1 Victor Toso 2013-06-05 03:56:55 UTC
Created attachment 246040 [details] [review]
Removes the internet check

(In reply to comment #0)
> Would it be possible to move the DNS lookup out of plugin init? Network can
> fail (or reappear) at any time, so having the check in plugin init doesn't
> really ensure all following network operation are going to succeed.

You are right. This check doesn't make sense.
The patch removes this check and the code for it.

When application try to make a search/browse without the database and internet, will simply fail with error.
Comment 2 Bastien Nocera 2013-06-05 09:19:58 UTC
(In reply to comment #1)
> Created an attachment (id=246040) [details] [review]
> Removes the internet check
> 
> (In reply to comment #0)
> > Would it be possible to move the DNS lookup out of plugin init? Network can
> > fail (or reappear) at any time, so having the check in plugin init doesn't
> > really ensure all following network operation are going to succeed.
> 
> You are right. This check doesn't make sense.
> The patch removes this check and the code for it.

The plugin init should *never* have failed, certainly.

> When application try to make a search/browse without the database and internet,
> will simply fail with error.

Why don't you use the "network-available" property of GNetworkMonitor? You can make the source appear when the network becomes available again.
Comment 3 Victor Toso 2013-06-06 01:42:54 UTC
(In reply to comment #2)
> Why don't you use the "network-available" property of GNetworkMonitor? You can
> make the source appear when the network becomes available again.

The idea was being sure that if there is no database, we would have internet to download it (when user interact with). For that, would be necessary check if www.magnatune.com is reachable.

But this check is not made in others plugins (like Youtube or Jamendo). The same approach was used (make the plugin visible, but if user try to reach content without internet just fail with error).
Comment 4 Juan A. Suarez Romero 2013-06-06 14:14:49 UTC
The idea of using GNetworkMonitor is interesting. In fact, I think it's interesting for all the plugins, not only the Magnatune one.

For now, I think I'm going to accept the current patch.
Comment 5 Juan A. Suarez Romero 2013-06-06 14:20:19 UTC
commit cb432d7e87c661240baf3cba712811f60278678a
Author: Victor Toso <me@victortoso.com>
Date:   Wed Jun 5 00:34:02 2013 -0300

    magnatune: Do not check for network connectivity
    
    It fixex huge delay resolving dns.
    
    g_network_monitor_can_reach() is sync and takes up to 30 seconds to
    resolve dns with some users.
    
    Link: https://bugzilla.gnome.org/show_bug.cgi?id=701336

 src/magnatune/grl-magnatune.c | 24 ------------------------
 1 file changed, 24 deletions(-)
Comment 6 Bastien Nocera 2013-08-23 15:51:16 UTC
*** Bug 706615 has been marked as a duplicate of this bug. ***