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 <[email protected]> wrote: > Gonzalo Diethelm <[email protected]> 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 <[email protected]> > Date: Thu Aug 6 15:10:34 2015 +0200 > > Added more files > > commit cc85e256666a826d3247e72ea39c5332bff721bf (refs/bisect/skip- > cc85e256666a826d3247e72ea39c5332bff721bf) > Author: Georg Richter <[email protected]> > Date: Thu Aug 6 15:08:25 2015 +0200 > > Added missing cio components > > commit f886562fb2c411bc7ff870d75a872d906674015b (refs/bisect/skip- > f886562fb2c411bc7ff870d75a872d906674015b) > Author: Georg Richter <[email protected]> > 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 [email protected]
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
