Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ping() on disconnected db handles #306

Open
ufobat opened this issue Mar 5, 2019 · 15 comments
Open

ping() on disconnected db handles #306

ufobat opened this issue Mar 5, 2019 · 15 comments

Comments

@ufobat
Copy link

ufobat commented Mar 5, 2019

With the latest version of DBD::mysql one of my testcases is failing. The testcase is basically just doing this

$dbh->disconnect();
ok( ! $dbh->ping(), 'dbh is diconnected');

We tracked this behaviour down to the commit 998cd6a, which is reopening a diconnected DBH.

The POD of DBI for ping says:

Attempts to determine, in a reasonably efficient way, if the database server is still running and the connection to it is still working

I am not perfectly sure but i would argue that ping should not reopen the connection.

@Grinnz
Copy link
Contributor

Grinnz commented Mar 5, 2019

Implicit reconnection is definitely a bad idea!

@Grinnz
Copy link
Contributor

Grinnz commented Mar 5, 2019

@dveeden there is a configuration already to enable this behavior mysql_auto_reconnect which is highly discouraged because it is bad to have a handle reconnect while you think you are in a transaction, or have table locks, or other session state that would be lost when reconnecting. This should not be done without this option set!

@hubandr
Copy link

hubandr commented Oct 7, 2019

Hi. Is there any reason why a implicit reconnect was implemented in version 4.050? Can't find any change documentation / requirements.

@dveeden
Copy link
Collaborator

dveeden commented Oct 7, 2019

Yes there was a reason to do that, but I don't recall the exact reason from the top of my head.
I agree that a ping should be done to keep the connection alive, not to re-connect.

However with mysql_auto_reconnect and a lot of historic behaviour this is probably not how it is working today.

@hashbangperl
Copy link

This causes a segfault with mariadb versions 10.3 and higher, if you call ping after disconnect.

use Test::More;
use DBI;

say "please provide db name";
my $dbname = ;
chomp $dbname;
say "please provide username";
my $user = ;
chomp $user;
say "please provide password";
my $password = ;
chomp $password;
my $dbh = DBI->connect("dbi:mysql:$dbname;host=localhost",$user, $password);
$dbh->disconnect();
ok( ! $dbh->ping(), 'dbh is disconnected and did not segv');

done_testing();

@hashbangperl
Copy link

I've been able to implement a workaround where disconnect is wrapped in a sub, and a flag set if the database version contains mariadb and version is 10.3 or higher, then checking that flag and forcing reconnect without calling ping if that flag is set (and unsetting the flag on successful connection)

@pali
Copy link
Member

pali commented Dec 6, 2019

Hello, just to note that DBI:MariaDB driver has fixed this problem and works fine with MariaDB 10.3.

@hashbangperl
Copy link

Hello, just to note that DBI:MariaDB driver has fixed this problem and works fine with MariaDB 10.3.

Hi Pali, no DBD::MariaDB does not fix the ping following disconnect segfault. I've replicated it on debian buster and stretch with both DBD::MariaDB and DBD::mysql against MariaDB 10.3 and 10.4 - both needed the workaround

@pali
Copy link
Member

pali commented Dec 6, 2019

@hashbangperl if there is still segfault with DBD::MariaDB please report it to our project https://github.com/gooddata/DBD-MariaDB/issues and we look at it... but it is strange as I was testing specially this problem against 10.3 and there was no crash anymore.

@hashbangperl
Copy link

@hashbangperl if there is still segfault with DBD::MariaDB please report it to our project https://github.com/gooddata/DBD-MariaDB/issues and we look at it... but it is strange as I was testing specially this problem against 10.3 and there was no crash anymore.

created issue. yeah, I got the problem between 1 in 2 and 1 in 4 times when running, and with the autoreconnect flag set.

@hashbangperl
Copy link

I've managed to trigger a segfault inside gdb with mysql dbd against mariadb, using the test above.
I was working the machine hard, as it seems to happen when mariadb is garbage collecting and under memory constraints.

gdb -ex r -ex bt -ex quit --args perl mariadb_disconnect_bug.t
GNU gdb (Debian 8.2.1-2+b3) 8.2.1
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
http://www.gnu.org/software/gdb/bugs/.
Find the GDB manual and other documentation resources online at:
http://www.gnu.org/software/gdb/documentation/.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from perl...(no debugging symbols found)...done.
Starting program: /opt/perl5/bin/perl mariadb_disconnect_bug.t
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1"

Program received signal SIGSEGV, Segmentation fault.
0x0000555555dfe910 in ?? ()
#0 0x0000555555dfe910 in ?? ()
#1 0x00007ffff7897c7d in mariadb_reconnect () from /usr/lib/x86_64-linux-gnu/libmariadb.so.3
#2 0x00007ffff7898379 in ?? () from /usr/lib/x86_64-linux-gnu/libmariadb.so.3
#3 0x00007ffff7896f20 in mysql_ping () from /usr/lib/x86_64-linux-gnu/libmariadb.so.3
#4 0x00007ffff78d952c in XS_DBD__mysql__db_ping (cv=) at mysql.xs:524
#5 0x00007ffff78fb9b8 in XS_DBI_dispatch () from /opt/perl5/lib/site_perl/5.26.3/x86_64-linux/auto/DBI/DBI.so
#6 0x0000555555624e96 in Perl_pp_entersub ()
#7 0x000055555561d1b3 in Perl_runops_standard ()
#8 0x00005555555a6eb4 in perl_run ()
#9 0x0000555555582192 in main ()
A debugging session is active.

Inferior 1 [process 7980] will be killed.

@hashbangperl
Copy link

added PR with extra tests, which should fail if using mariadb 10.3 or newer server that's under load. #319

bestpractical-mirror pushed a commit to bestpractical/dbix-searchbuilder that referenced this issue Jun 30, 2020
We call ping on existing dbh to check if it's still active or not, which
sadly could cause segmentation faults on mariadb 10.3+, related code is
in both Connect and TransactionDepth methods:

    $self->dbh && $self->dbh->ping

Here we unset dbh to get around the issue.

A couple of more notes:

* Not just ping, SQL queries also have this issue.

Thus we can't change ping to a light query like "SELECT VERSION()"

* It happens only on an *explicitly* disconnected handle

There is no such issue for indirect disconnects like restarting database
server or killing connection processes.

See also perl5-dbi/DBD-mysql#306
bestpractical-mirror pushed a commit to bestpractical/dbix-searchbuilder that referenced this issue Jul 1, 2020
We call ping on existing dbh to check if it's still active or not, which
sadly could cause segmentation faults on mariadb 10.2+, related code is
in both Connect and TransactionDepth methods:

    $self->dbh && $self->dbh->ping

Here we unset dbh to get around the issue.

A couple of more notes:

* Not just ping, SQL queries also have this issue.

Thus we can't change ping to a light query like "SELECT VERSION()"

* It happens only on an *explicitly* disconnected handle

There is no such issue for indirect disconnects like restarting database
server or killing connection processes.

See also perl5-dbi/DBD-mysql#306
@dfskoll
Copy link

dfskoll commented Jul 31, 2020

Hi, all,

I've had a chance to look at this briefly. Here is where the code is segfaulting in mariadb_lib.c:

  /* check if connection handler is active */
  if (IS_CONNHDLR_ACTIVE(mysql))
  {
    if (mysql->extension->conn_hdlr->plugin && mysql->extension->conn_hdlr->plugin->reconnect)
      return(mysql->extension->conn_hdlr->plugin->reconnect(mysql));  // SEGFAULT
  }

I believe the error is in mysql_close in mariadb_lib.c. Have a look at this code:

    if (mysql->net.extension)
      free(mysql->net.extension);

    mysql->host_info=mysql->user=mysql->passwd=mysql->db=0;

    /* Clear pointers for better safety */
    memset((char*) &mysql->options, 0, sizeof(mysql->options));

    if (mysql->extension)
      free(mysql->extension);

mysql_close frees msyql->net.extension and mysql->extension, but does NOT set them to NULL. So they're pointing to freed memory and when they are reused... SEGV.

I will try and patch mariadb_lib.c to see if my theory is correct and if I can fix it.

Regards,

Dianne.

@dfskoll
Copy link

dfskoll commented Jul 31, 2020

This patch to the MariaDB client library seems to fix the problem.
patch.txt

@svetasmirnova
Copy link

I believe the patch that caused this needs to be reverted and implemented properly. Currently, the workaround to return the previous behavior is to use AutoCommit = 0. This is absolutely wrong, and the said patch should check mysql_auto_reconnect instead.

svetasmirnova added a commit to percona/percona-toolkit that referenced this issue Mar 17, 2023
- Updated test t/lib/TableChecksum.t so it reflects changes, introduced in the fix for PT-2016
- Updated test t/lib/RowChecksum.t so it reflects changes added to in the fix for PT-2138: UTF8 support
- Uncommented SQL in sandbox/slave_channels.sql that made t/lib/MasterSlave.t to fail
- Added check for undef into t/pt-archiver/archive_using_channels.t
- Updated lib/Cxn.pm so it uses $dbh->{Active} after issue with the ping() call, reported at perl5-dbi/DBD-mysql#306
svetasmirnova added a commit to percona/percona-toolkit that referenced this issue Mar 17, 2023
- Impoved the fix for PT-2016, so it does not files with keys with USING keyword
- Added brackets to expression in lib/TableNibbler.pm, so it does not crap query wit many indexes with OR keyword
- Adjusted test t/lib/TableNibbler.t, so it reflects above chages
- Modified lib/Cxn.pm, so it has workaround for perl5-dbi/DBD-mysql#306 , introduced in DBD::mysql 4.0.50
- Updated tests: added debugging code and cleanups
- Updated modules for tools
svetasmirnova added a commit to percona/percona-toolkit that referenced this issue Mar 27, 2023
* PT-2156 - Fix tests for lib

- Fixed tests broken for lib/TableParser.pm after fix for PT-1059
- Updated tests for lib/TableParser.pm that are broken due to SHOW CREATE TABLE output format change in 8.0
- Updated modules for all tools that use lib/TableParser.pm

* Revert "Fixed pt-archiver tests"

This reverts commit a3ab87b.

This commit wa needed, because removed code in sandbox/slave_channels.sql broked the test. Proper fix would be to do not remove channel names rather than chaging the test. So revertig it.

* PT-2156 - Fix tests for lib

- Updated test t/lib/TableChecksum.t so it reflects changes, introduced in the fix for PT-2016
- Updated test t/lib/RowChecksum.t so it reflects changes added to in the fix for PT-2138: UTF8 support
- Uncommented SQL in sandbox/slave_channels.sql that made t/lib/MasterSlave.t to fail
- Added check for undef into t/pt-archiver/archive_using_channels.t
- Updated lib/Cxn.pm so it uses $dbh->{Active} after issue with the ping() call, reported at perl5-dbi/DBD-mysql#306

* PT-2156 - Fix tests for lib

- Impoved the fix for PT-2016, so it does not files with keys with USING keyword
- Added brackets to expression in lib/TableNibbler.pm, so it does not crap query wit many indexes with OR keyword
- Adjusted test t/lib/TableNibbler.t, so it reflects above chages
- Modified lib/Cxn.pm, so it has workaround for perl5-dbi/DBD-mysql#306 , introduced in DBD::mysql 4.0.50
- Updated tests: added debugging code and cleanups
- Updated modules for tools
@dveeden dveeden changed the title ping() on disconneted db handles ping() on disconnected db handles Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants