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 744720 - Add a SoupMessage flag to ignore connection limits
Add a SoupMessage flag to ignore connection limits
Status: RESOLVED FIXED
Product: libsoup
Classification: Core
Component: HTTP Transport
2.49.x
Other Linux
: Normal normal
: ---
Assigned To: libsoup-maint@gnome.bugs
libsoup-maint@gnome.bugs
Depends on: 731153
Blocks:
 
 
Reported: 2015-02-18 16:46 UTC by Carlos Garcia Campos
Modified: 2015-03-02 08:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add SoupMessage flag SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS (12.87 KB, patch)
2015-02-18 16:47 UTC, Carlos Garcia Campos
none Details | Review
Updated patch (10.73 KB, patch)
2015-02-23 10:03 UTC, Carlos Garcia Campos
committed Details | Review

Description Carlos Garcia Campos 2015-02-18 16:46:32 UTC
In some situation, we need to ensure a particular message is processed even if there aren't connections available because one of any of the connection limits. In such case we could flag the message soe that a new connection is created for it and dropped when the message finishes.
Comment 1 Carlos Garcia Campos 2015-02-18 16:47:41 UTC
Created attachment 297113 [details] [review]
Add SoupMessage flag SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS
Comment 2 Dan Winship 2015-02-19 23:10:20 UTC
Comment on attachment 297113 [details] [review]
Add SoupMessage flag SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS

>+		if (item->conn_is_dedicated)
>+			dedicated_conn = g_object_ref (item->conn);

I feel like you shouldn't need conn_is_dedicated... you could just always close the connection at the end if and only if priv->num_conns or host->num_conns is over the limit.

Although... I can see how that might end up being more complicated than it seems at first because of locking and stuff... is that what you were running into before when you said you were having trouble getting it working right?


At any rate, you don't need the new argument to soup_session_set_item_connection(); just always set it FALSE there, and then set it TRUE only in the one place where it might be TRUE in get_connection().
Comment 3 Carlos Garcia Campos 2015-02-23 09:57:01 UTC
(In reply to Dan Winship from comment #2)
> Comment on attachment 297113 [details] [review] [review]
> Add SoupMessage flag SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS
> 
> >+		if (item->conn_is_dedicated)
> >+			dedicated_conn = g_object_ref (item->conn);
> 
> I feel like you shouldn't need conn_is_dedicated... you could just always
> close the connection at the end if and only if priv->num_conns or
> host->num_conns is over the limit.

hmm, it's true that at that point another idle connection could have been cleaned up, and we don't need to drop this dedicated connection. The presence of ignore-conn flag doesn't ensure that the connection used was created ignoring the limits, but it doesn't matter if we only drop the connection when current number of connections are over the limits. If there are different messages with the flag, we would need to check if the current limits apply to the message or not, because there might be connections to different hosts. So, I think at a first step, knowing that the connection is dedicated and dropping it is indeed the safest and easiest solution. We can improve it later I guess. In the particular case of WebKit this will only be used for sync XHR, so we know that when this message finishes, no other message was running, and the current num conns is already over any of the limits, and we need to drop that connection. 

> Although... I can see how that might end up being more complicated than it
> seems at first because of locking and stuff... is that what you were running
> into before when you said you were having trouble getting it working right?

My problems had more to do with the differences in the lifecycle of cached/non-cached resources, and knowing what connections to drop when the limits change (since we were changing the limits before).

> 
> At any rate, you don't need the new argument to
> soup_session_set_item_connection(); just always set it FALSE there, and then
> set it TRUE only in the one place where it might be TRUE in get_connection().

Fair enough.
Comment 4 Carlos Garcia Campos 2015-02-23 10:03:18 UTC
Created attachment 297629 [details] [review]
Updated patch

Removed the parameter of soup_session_set_item_connection and added a FIXME to only drop the connection if the current conns are still over the limits at that point.
Comment 5 Carlos Garcia Campos 2015-03-02 08:47:29 UTC
Comment on attachment 297629 [details] [review]
Updated patch

Pushed.