You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2012/06/19 15:54:45 UTC

svn commit: r1351717 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h serf.c

Author: rhuijben
Date: Tue Jun 19 13:54:45 2012
New Revision: 1351717

URL: http://svn.apache.org/viewvc?rev=1351717&view=rev
Log:
* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__session_t): Make timeout type match the argument type of
    serf_context_run, its only user.

* subversion/libsvn_ra_serf/serf.c
  (load_config): Verify that we don't set timeout to a negative value.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/serf.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1351717&r1=1351716&r2=1351717&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Tue Jun 19 13:54:45 2012
@@ -189,7 +189,7 @@ struct svn_ra_serf__session_t {
   const char *uuid;
 
   /* Connection timeout value */
-  long timeout;
+  apr_short_interval_time_t timeout;
 
   /* HTTPv1 flags */
   svn_tristate_t supports_deadprop_count;

Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=1351717&r1=1351716&r2=1351717&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Tue Jun 19 13:54:45 2012
@@ -270,6 +270,11 @@ load_config(svn_ra_serf__session_t *sess
   else
     session->timeout = apr_time_from_sec(DEFAULT_HTTP_TIMEOUT);
 
+  if (session->timeout < 0) /* Always true for DEFAULT_HTTP_TIMEOUT */
+    session->timeout = apr_time_from_sec(600); /* 10 min */
+
+  SVN_ERR_ASSERT(session->timeout > 0);
+
   /* Convert the proxy port value, if any. */
   if (port_str)
     {



RE: svn commit: r1351717 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h serf.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: rhuijben@apache.org [mailto:rhuijben@apache.org]
> Sent: dinsdag 19 juni 2012 15:55
> To: commits@subversion.apache.org
> Subject: svn commit: r1351717 - in
> /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h serf.c
> 
> Author: rhuijben
> Date: Tue Jun 19 13:54:45 2012
> New Revision: 1351717
> 
> URL: http://svn.apache.org/viewvc?rev=1351717&view=rev
> Log:
> * subversion/libsvn_ra_serf/ra_serf.h
>   (svn_ra_serf__session_t): Make timeout type match the argument type of
>     serf_context_run, its only user.
> 
> * subversion/libsvn_ra_serf/serf.c
>   (load_config): Verify that we don't set timeout to a negative value.

[Not sure when this message gets through. My ADSL is still broken]

Unless the user configured a different timeout value in his/her configuration we used to set timeout to a negative value on any platform with 32 bit longs. This patch should make the behavior the same on all platforms.

This timeout is probably still too long, but setting it negative made it infinite.

The code should be fixed to use a proper default. 
Waiting for 10 minutes until checking for ^C via the cancel function is still broken behavior for GUI clients like AnkhSVN.

I see no problem in waiting for more than 10 minutes, but it should be user cancelable within at least something like 30 seconds, preferably sooner or the user will just kill his application.
(Which in case of AnkhSVN will include losing work in open editors)

	Bert



RE: svn commit: r1351717 - in /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h serf.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: rhuijben@apache.org [mailto:rhuijben@apache.org]
> Sent: dinsdag 19 juni 2012 15:55
> To: commits@subversion.apache.org
> Subject: svn commit: r1351717 - in
> /subversion/trunk/subversion/libsvn_ra_serf: ra_serf.h serf.c
> 
> Author: rhuijben
> Date: Tue Jun 19 13:54:45 2012
> New Revision: 1351717
> 
> URL: http://svn.apache.org/viewvc?rev=1351717&view=rev
> Log:
> * subversion/libsvn_ra_serf/ra_serf.h
>   (svn_ra_serf__session_t): Make timeout type match the argument type of
>     serf_context_run, its only user.
> 
> * subversion/libsvn_ra_serf/serf.c
>   (load_config): Verify that we don't set timeout to a negative value.

[Not sure when this message gets through. My ADSL is still broken]

Unless the user configured a different timeout value in his/her configuration we used to set timeout to a negative value on any platform with 32 bit longs. This patch should make the behavior the same on all platforms.

This timeout is probably still too long, but setting it negative made it infinite.

The code should be fixed to use a proper default. 
Waiting for 10 minutes until checking for ^C via the cancel function is still broken behavior for GUI clients like AnkhSVN.

I see no problem in waiting for more than 10 minutes, but it should be user cancelable within at least something like 30 seconds, preferably sooner or the user will just kill his application.
(Which in case of AnkhSVN will include losing work in open editors)

	Bert