maria-developers team mailing list archive

[Gonzalo Diethelm <gonzalo.diethelm@xxxxxxxxx>]

Hi Kristian,

Well, that's awesome. Thanks for taking the time to dig into this. I have
added the workaround you suggested (ignoring timeouts with a zero value),
and on Monday I will check the connector github repo and test again.

Best regards,
Gonzalo


On Sat, Sep 24, 2016 at 1:42 PM, Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>
wrote:

> Gonzalo Diethelm <gonzalo.diethelm@xxxxxxxxx> writes:
>
> > $ brew info mariadb-connector-c
> > mariadb-connector-c: stable 2.2.2 (bottled)
> > MariaDB database connector for C applications
> > https://downloads.mariadb.org/connector-c/
>
> Ok, so this is the separate client library, not the one in the server.
>
> > Notice that in the code snippet you sent, the comparison is greater than
> > *or equal to* zero; this could in fact add a zero timeout, right?
> >
> >     if (vio_timeout >= 0)
> >     {
> >       b->timeout_value= vio_timeout;
> >       b->events_to_wait_for|= MYSQL_WAIT_TIMEOUT;
> >     }
>
> Yes, you are right of course. But now I see it: For the application, zero
> means no timeout. Internally in the code, this is converted to -1 meaning
> no
> timeout.
>
> And while this looks correct in the server library, the connector-c
> standalone library seems to do this incorrectly:
>
>   void vio_read_timeout(Vio *vio, uint timeout)
>   {
>     vio->read_timeout= (timeout >= 0) ? timeout * 1000 : -1;
>   }
>
>
>   int vio_timeout= (mysql->options.connect_timeout >= 0) ?
>                    mysql->options.connect_timeout * 1000 : -1;
>
> This code is comparing an unsigned value >= 0; the comparison should be >0
> (an unsigned value is always >= 0, so this is obviously wrong).
>
> So I was able to reproduce your problem with version 2.2.2 of the
> standalone
> library. However, recent changes seem to have fixed it, I could no longer
> reproduce. In particular it seems fixed after these three commits which
> seem
> to rewrite the I/O part of the library:
>
> -----------------------------------------------------------------------
> commit a349bee53d38ee133b949b20f84ccc29d3aa4465 (refs/bisect/bad)
> Author: Georg Richter <georg@xxxxxxxxxxx>
> Date:   Thu Aug 6 15:10:34 2015 +0200
>
>     Added more files
>
> commit cc85e256666a826d3247e72ea39c5332bff721bf (refs/bisect/skip-
> cc85e256666a826d3247e72ea39c5332bff721bf)
> Author: Georg Richter <georg@xxxxxxxxxxx>
> Date:   Thu Aug 6 15:08:25 2015 +0200
>
>     Added missing cio components
>
> commit f886562fb2c411bc7ff870d75a872d906674015b (refs/bisect/skip-
> f886562fb2c411bc7ff870d75a872d906674015b)
> Author: Georg Richter <georg@xxxxxxxxxxx>
> Date:   Thu Aug 6 13:06:54 2015 +0200
>
>     Initial cio implementation
> -----------------------------------------------------------------------
>
> Wlad, Georg, what are the plans for Connector-C? Will the current master
> branch be released as an update for the stand-alone client library? Or only
> as part of 10.2?
>
> In the latter case, do you agree that a bugfix like the below patch is
> needed for existing stable releases of Connector-C? Since it seems that
> without it, timeout for non-blocking connects (and read/writes?) is
> completely broken?
>
> Gonzola, thanks for bringing this to attention! If possible, you could try
> checking the latest git source code if your problem is indeed solved.
>
> Otherwise, one workaround until a fix is released is to use the
> libmysqlclient supplied as part of the MariaDB server release.
>
> Alternatively, change your code to interpret MYSQL_WAIT_TIMEOUT with a zero
> value from mysql_get_timeout_value() the same as no MYSQL_WAIT_TIMEOUT.
> This
> change will still be correct (though redundant) after the library is fixed,
> as MYSQL_WAIT_TIMEOUT can never legally have a zero timeout.
>
> Thanks,
>
>  - Kristian.
>
>
> diff --git a/libmariadb/libmariadb.c b/libmariadb/libmariadb.c
> index 8fd87ba..23aa407 100644
> --- a/libmariadb/libmariadb.c
> +++ b/libmariadb/libmariadb.c
> @@ -254,7 +254,7 @@ static int
>  connect_sync_or_async(MYSQL *mysql, NET *net, my_socket fd,
>                        const struct sockaddr *name, uint namelen)
>  {
> -  int vio_timeout= (mysql->options.connect_timeout >= 0) ?
> +  int vio_timeout= (mysql->options.connect_timeout > 0) ?
>                     mysql->options.connect_timeout * 1000 : -1;
>
>    if (mysql->options.extension && mysql->options.extension->async_context
> &&
> @@ -1767,7 +1767,7 @@ MYSQL *mthd_my_real_connect(MYSQL *mysql, const char
> *host, const char *user,
>      vio_write_timeout(net->vio, mysql->options.read_timeout);
>    /* Get version info */
>    mysql->protocol_version= PROTOCOL_VERSION;   /* Assume this */
> -  if (mysql->options.connect_timeout >= 0 &&
> +  if (mysql->options.connect_timeout > 0 &&
>        vio_wait_or_timeout(net->vio, FALSE, mysql->options.connect_timeout
> * 1000) < 1)
>    {
>      my_set_error(mysql, CR_SERVER_LOST, SQLSTATE_UNKNOWN,
> diff --git a/libmariadb/violite.c b/libmariadb/violite.c
> index 1fbdcc3..722724f 100644
> --- a/libmariadb/violite.c
> +++ b/libmariadb/violite.c
> @@ -127,13 +127,13 @@ void vio_timeout(Vio *vio, int type, uint timeval)
>
>  void vio_read_timeout(Vio *vio, uint timeout)
>  {
> -  vio->read_timeout= (timeout >= 0) ? timeout * 1000 : -1;
> +  vio->read_timeout= (timeout > 0) ? timeout * 1000 : -1;
>    vio_timeout(vio, SO_RCVTIMEO, vio->read_timeout);
>  }
>
>  void vio_write_timeout(Vio *vio, uint timeout)
>  {
> -  vio->write_timeout= (timeout >= 0) ? timeout * 1000 : -1;
> +  vio->write_timeout= (timeout > 0) ? timeout * 1000 : -1;
>    vio_timeout(vio, SO_SNDTIMEO, vio->write_timeout);
>  }
>
>
>


-- 
Gonzalo Diethelm
gonzalo.diethelm@xxxxxxxxx