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 703463 - Connections sometimes aren’t cleaned up
Connections sometimes aren’t cleaned up
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: HTTP Transport
unspecified
Other All
: Normal major
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
: 698912 701127 702747 (view as bug list)
Depends on:
Blocks: 703515
 
 
Reported: 2013-07-02 14:07 UTC by Philip Withnall
Modified: 2013-09-13 14:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Force connection cleanup if exceeding max connections per host (1.20 KB, patch)
2013-07-02 14:09 UTC, Philip Withnall
needs-work Details | Review
Force cleanup of REMOTE_DISCONNECTED conns for sync sessions (1.13 KB, patch)
2013-07-03 18:58 UTC, Philip Withnall
committed Details | Review

Description Philip Withnall 2013-07-02 14:07:27 UTC
With some work I’m doing on libgdata at the moment, I can reliably get libsoup into a situation where it has run out of connections, but won’t clean any up when trying to set up a new one, and so it hangs.

I’m not familiar enough with the libsoup code to work out the cause of this, but I have a patch which fixes the problem, and touches some suspect-looking code.

This is reproducible with libsoup e06d243fadb351b63ba6e7734fd1a9b9fccf4d50 and some local modifications to libgdata (such as using SoupSession instead of SoupSessionSync) which I can provide if necessary.
Comment 1 Philip Withnall 2013-07-02 14:09:59 UTC
Created attachment 248226 [details] [review]
Force connection cleanup if exceeding max connections per host

This makes the two branches which compare numbers of connections have identical behaviour, which seems less suspect to me than the previous implementation. I don’t know anything about this code though, so I could be wildly wrong.
Comment 2 Dan Winship 2013-07-02 20:49:35 UTC
Comment on attachment 248226 [details] [review]
Force connection cleanup if exceeding max connections per host

See also bug 698912, which links to https://bugzilla.redhat.com/show_bug.cgi?id=976529, which is an evolution hang in libgdata, which is presumably the same bug you're seeing (though still using SoupSessionSync, not SoupSession).

This patch is wrong, but based on the fact that it fixes your bug, I think I've figured out the right fix. If you add

    soup_session_cleanup_connections (session, FALSE);

to the top of get_connection(), does that fix it?

(The problem, I think, is with connections in the REMOTE_DISCONNECTED state. The code is implicitly assuming that they get cleaned up now and then by someone else, which happens for async sessions, but not sync ones. Your patch fixed that, but also causes it to disconnect IDLE connections more often than is strictly necessary. This patch should only clean up the REMOTE_DISCONNECTED ones.)
Comment 3 Philip Withnall 2013-07-03 18:58:53 UTC
Created attachment 248345 [details] [review]
Force cleanup of REMOTE_DISCONNECTED conns for sync sessions

Yup, that does the trick. Updated patch attached. Hopefully the commit message accurately describes things; I just reworded your explanation above.
Comment 4 Dan Winship 2013-07-05 14:54:07 UTC
Comment on attachment 248345 [details] [review]
Force cleanup of REMOTE_DISCONNECTED conns for sync sessions

looks good
Comment 5 Philip Withnall 2013-07-05 17:55:30 UTC
Comment on attachment 248345 [details] [review]
Force cleanup of REMOTE_DISCONNECTED conns for sync sessions

Committed.

commit 2081c31235e7c2b3a15fa0a819631106cf4c3efa
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Tue Jul 2 15:04:18 2013 +0100

    soup-session: Force cleanup of REMOTE_DISCONNECTED conns for sync sessions
    
    get_connection() assumed that REMOTE_DISCONNECTED connections would get
    implicitly cleaned up elsewhere in the code, which is true for async
    sessions, but not sync ones. This could lead to connection exhaustion and
    hence hangs.
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=703463

 libsoup/soup-session.c | 2 ++
 1 file changed, 2 insertions(+)
Comment 6 Matthew Barnes 2013-07-07 19:31:34 UTC
*** Bug 702747 has been marked as a duplicate of this bug. ***
Comment 7 Dan Winship 2013-07-12 21:02:51 UTC
*** Bug 698912 has been marked as a duplicate of this bug. ***
Comment 8 Dan Winship 2013-08-10 16:25:13 UTC
*** Bug 701127 has been marked as a duplicate of this bug. ***
Comment 9 David Woodhouse 2013-09-13 14:04:23 UTC
Now also fixed in gnome-3-8 branch:
 https://git.gnome.org/browse/libsoup/commit/?h=gnome-3-8&id=c764b854