Skip to content

Commit 9c0a139

Browse files
palipali
committed
Merge pull request #131
Fix disconnect_all() method to work also during global destruction phase
2 parents f5eb73a + b9f5b14 commit 9c0a139

File tree

4 files changed

+125
-98
lines changed

4 files changed

+125
-98
lines changed

‎MANIFEST

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ t/10connect.t
1919
t/11data_sources.t
2020
t/12embedded.t
2121
t/13disconnect.t
22+
t/14destruct.t
2223
t/15reconnect.t
2324
t/16dbi-get_info.t
2425
t/20createdrop.t

‎dbdimp.c

Lines changed: 66 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -2291,6 +2291,8 @@ static bool mariadb_dr_connect(
22912291

22922292
imp_dbh->async_query_in_flight = NULL;
22932293

2294+
mariadb_list_add(imp_drh->active_imp_dbhs, imp_dbh->list_entry, imp_dbh);
2295+
22942296
return TRUE;
22952297
}
22962298

@@ -2336,8 +2338,7 @@ static bool mariadb_db_my_login(pTHX_ SV* dbh, imp_dbh_t *imp_dbh)
23362338
char* user;
23372339
char* password;
23382340
char* mysql_socket;
2339-
I32 i;
2340-
bool found_taken_pmysql;
2341+
struct mariadb_list_entry *entry;
23412342
D_imp_xxh(dbh);
23422343
D_imp_drh_from_dbh;
23432344

@@ -2347,34 +2348,29 @@ static bool mariadb_db_my_login(pTHX_ SV* dbh, imp_dbh_t *imp_dbh)
23472348
{
23482349
if (DBIc_TRACE_LEVEL(imp_xxh) >= 2)
23492350
PerlIO_printf(DBIc_LOGPIO(imp_xxh), "mariadb_db_my_login skip connect\n");
2351+
23502352
/* tell our parent we've adopted an active child */
23512353
++DBIc_ACTIVE_KIDS(DBIc_PARENT_COM(imp_dbh));
2352-
found_taken_pmysql = FALSE;
2353-
if (imp_drh->taken_pmysqls)
2354+
2355+
for (entry = imp_drh->taken_pmysqls; entry; entry = entry->next)
23542356
{
2355-
for (i = AvFILL(imp_drh->taken_pmysqls); i >= 0; --i)
2357+
if ((MYSQL *)entry->data == imp_dbh->pmysql)
23562358
{
2357-
svp = av_fetch(imp_drh->taken_pmysqls, i, FALSE);
2358-
if (!svp || !*svp)
2359-
continue;
2360-
SvGETMAGIC(*svp);
2361-
if (!SvIOK(*svp))
2362-
continue;
2363-
if (imp_dbh->pmysql == INT2PTR(MYSQL *, SvIVX(*svp)))
2364-
{
2365-
found_taken_pmysql = TRUE;
2366-
av_delete(imp_drh->taken_pmysqls, i, G_DISCARD);
2367-
break;
2368-
}
2359+
/* Remove MYSQL* entry from taken list */
2360+
mariadb_list_remove(imp_drh->taken_pmysqls, entry);
2361+
2362+
/* Add imp_dbh entry into active_imp_dbhs list */
2363+
mariadb_list_add(imp_drh->active_imp_dbhs, imp_dbh->list_entry, imp_dbh);
2364+
2365+
return TRUE;
23692366
}
23702367
}
2371-
if (!found_taken_pmysql)
2372-
{
2373-
imp_dbh->pmysql = NULL;
2374-
mariadb_dr_do_error(dbh, CR_CONNECTION_ERROR, "Connection error: dbi_imp_data is not valid", "HY000");
2375-
return FALSE;
2376-
}
2377-
return TRUE;
2368+
2369+
/* This imp_dbh data belongs to different connection, so destructor should not touch it */
2370+
imp_dbh->list_entry = NULL;
2371+
imp_dbh->pmysql = NULL;
2372+
mariadb_dr_do_error(dbh, CR_CONNECTION_ERROR, "Connection error: dbi_imp_data is not valid", "HY000");
2373+
return FALSE;
23782374
}
23792375
if (DBIc_TRACE_LEVEL(imp_xxh) >= 2)
23802376
PerlIO_printf(DBIc_LOGPIO(imp_xxh),
@@ -2445,12 +2441,15 @@ SV *mariadb_db_take_imp_data(SV *dbh, imp_xxh_t *imp_xxh, void *foo)
24452441
dTHX;
24462442
D_imp_dbh(dbh);
24472443
D_imp_drh_from_dbh;
2444+
struct mariadb_list_entry *entry;
24482445
PERL_UNUSED_ARG(imp_xxh);
24492446
PERL_UNUSED_ARG(foo);
24502447

2451-
if (!imp_drh->taken_pmysqls)
2452-
imp_drh->taken_pmysqls = newAV();
2453-
av_push(imp_drh->taken_pmysqls, newSViv(PTR2IV(imp_dbh->pmysql)));
2448+
/* Add MYSQL* into taken list */
2449+
mariadb_list_add(imp_drh->taken_pmysqls, entry, imp_dbh->pmysql);
2450+
2451+
/* MYSQL* was taken from imp_dbh so remove it also from active_imp_dbhs list */
2452+
mariadb_list_remove(imp_drh->active_imp_dbhs, imp_dbh->list_entry);
24542453

24552454
return &PL_sv_no;
24562455
}
@@ -2979,39 +2978,23 @@ static void mariadb_dr_close_mysql(pTHX_ imp_drh_t *imp_drh, MYSQL *pmysql)
29792978
}
29802979
}
29812980

2982-
/*
2983-
***************************************************************************
2984-
*
2985-
* Name: mariadb_db_disconnect
2986-
*
2987-
* Purpose: Disconnect a database handle from its database
2988-
*
2989-
* Input: dbh - database handle being disconnected
2990-
* imp_dbh - drivers private database handle data
2991-
*
2992-
* Returns: 1 for success (always)
2993-
*
2994-
**************************************************************************/
2995-
2996-
int mariadb_db_disconnect(SV* dbh, imp_dbh_t* imp_dbh)
2981+
static void mariadb_db_close_mysql(pTHX_ imp_drh_t *imp_drh, imp_dbh_t *imp_dbh)
29972982
{
2998-
dTHX;
29992983
AV *av;
30002984
I32 i;
30012985
MAGIC *mg;
30022986
SV **svp;
30032987
SV *sv;
30042988
SV *sth;
30052989
imp_sth_t *imp_sth;
3006-
D_imp_xxh(dbh);
3007-
D_imp_drh_from_dbh;
30082990

3009-
/* We assume that disconnect will always work */
3010-
/* since most errors imply already disconnected. */
2991+
if (DBIc_TRACE_LEVEL(imp_dbh) >= 2)
2992+
PerlIO_printf(DBIc_LOGPIO(imp_dbh), "\tmariadb_db_close_mysql: imp_dbh=%p pmysql=%p\n", imp_dbh, imp_dbh->pmysql);
2993+
30112994
DBIc_ACTIVE_off(imp_dbh);
3012-
if (DBIc_TRACE_LEVEL(imp_xxh) >= 2)
3013-
PerlIO_printf(DBIc_LOGPIO(imp_xxh), "imp_dbh->pmysql: %p\n",
3014-
imp_dbh->pmysql);
2995+
2996+
if (imp_dbh->list_entry)
2997+
mariadb_list_remove(imp_drh->active_imp_dbhs, imp_dbh->list_entry);
30152998

30162999
if (imp_dbh->pmysql)
30173000
{
@@ -3045,14 +3028,39 @@ int mariadb_db_disconnect(SV* dbh, imp_dbh_t* imp_dbh)
30453028
* CVE 2017-3302 do not do it. So do it manually to prevent crash. */
30463029
if (imp_sth->stmt && imp_sth->stmt->mysql)
30473030
{
3048-
if (DBIc_TRACE_LEVEL(imp_xxh) >= 2)
3049-
PerlIO_printf(DBIc_LOGPIO(imp_xxh), "Applying CVE 2017-3302 workaround for sth=0x%p\n", imp_sth);
3031+
if (DBIc_TRACE_LEVEL(imp_dbh) >= 2)
3032+
PerlIO_printf(DBIc_LOGPIO(imp_dbh), "Applying CVE 2017-3302 workaround for sth=%p\n", imp_sth);
30503033
imp_sth->stmt->mysql = NULL;
30513034
}
30523035
}
30533036
}
30543037
}
30553038
}
3039+
}
3040+
3041+
/*
3042+
***************************************************************************
3043+
*
3044+
* Name: mariadb_db_disconnect
3045+
*
3046+
* Purpose: Disconnect a database handle from its database
3047+
*
3048+
* Input: dbh - database handle being disconnected
3049+
* imp_dbh - drivers private database handle data
3050+
*
3051+
* Returns: 1 for success (always)
3052+
*
3053+
**************************************************************************/
3054+
3055+
int mariadb_db_disconnect(SV* dbh, imp_dbh_t* imp_dbh)
3056+
{
3057+
dTHX;
3058+
D_imp_drh_from_dbh;
3059+
PERL_UNUSED_ARG(dbh);
3060+
3061+
/* We assume that disconnect will always work */
3062+
/* since most errors imply already disconnected. */
3063+
mariadb_db_close_mysql(aTHX_ imp_drh, imp_dbh);
30563064

30573065
/* We don't free imp_dbh since a reference still exists */
30583066
/* The DESTROY method is the only one to 'free' memory. */
@@ -3075,57 +3083,18 @@ int mariadb_db_disconnect(SV* dbh, imp_dbh_t* imp_dbh)
30753083

30763084
int mariadb_dr_discon_all (SV *drh, imp_drh_t *imp_drh) {
30773085
dTHX;
3078-
dSP;
30793086
int ret;
3080-
SV **svp;
3081-
AV *av;
3082-
I32 i;
3087+
struct mariadb_list_entry *entry;
30833088
PERL_UNUSED_ARG(drh);
30843089

3085-
if (imp_drh->taken_pmysqls)
3090+
while ((entry = imp_drh->taken_pmysqls))
30863091
{
3087-
for (i = AvFILL(imp_drh->taken_pmysqls); i >= 0; --i)
3088-
{
3089-
svp = av_fetch(imp_drh->taken_pmysqls, i, FALSE);
3090-
if (!svp || !*svp)
3091-
continue;
3092-
SvGETMAGIC(*svp);
3093-
if (!SvIOK(*svp))
3094-
continue;
3095-
mariadb_dr_close_mysql(aTHX_ imp_drh, INT2PTR(MYSQL *, SvIVX(*svp)));
3096-
}
3097-
av_undef(imp_drh->taken_pmysqls);
3098-
imp_drh->taken_pmysqls = NULL;
3092+
mariadb_dr_close_mysql(aTHX_ imp_drh, (MYSQL *)entry->data);
3093+
mariadb_list_remove(imp_drh->taken_pmysqls, entry);
30993094
}
31003095

3101-
svp = hv_fetchs((HV*)DBIc_MY_H(imp_drh), "ChildHandles", FALSE);
3102-
if (svp && *svp)
3103-
{
3104-
SvGETMAGIC(*svp);
3105-
if (SvROK(*svp) && SvTYPE(SvRV(*svp)) == SVt_PVAV)
3106-
{
3107-
av = (AV *)SvRV(*svp);
3108-
for (i = AvFILL(av); i >= 0; --i)
3109-
{
3110-
svp = av_fetch(av, i, FALSE);
3111-
if (!svp || !*svp || !sv_isobject(*svp))
3112-
continue;
3113-
3114-
ENTER;
3115-
SAVETMPS;
3116-
3117-
PUSHMARK(SP);
3118-
EXTEND(SP, 1);
3119-
PUSHs(sv_2mortal(newSVsv(*svp)));
3120-
PUTBACK;
3121-
3122-
call_method("disconnect", G_VOID|G_DISCARD|G_EVAL|G_KEEPERR);
3123-
3124-
FREETMPS;
3125-
LEAVE;
3126-
}
3127-
}
3128-
}
3096+
while (imp_drh->active_imp_dbhs)
3097+
mariadb_db_close_mysql(aTHX_ imp_drh, (imp_dbh_t *)imp_drh->active_imp_dbhs->data);
31293098

31303099
ret = 1;
31313100

‎dbdimp.h

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,35 @@ enum av_attribs {
417417
}; /* purposes only */
418418

419419

420+
/* Double linked list */
421+
struct mariadb_list_entry {
422+
void *data;
423+
struct mariadb_list_entry *prev;
424+
struct mariadb_list_entry *next;
425+
};
426+
#define mariadb_list_add(list, entry, ptr) \
427+
STMT_START { \
428+
Newz(0, (entry), 1, struct mariadb_list_entry); \
429+
(entry)->data = (ptr); \
430+
(entry)->prev = NULL; \
431+
(entry)->next = (list); \
432+
if ((list)) \
433+
(list)->prev = (entry); \
434+
(list) = (entry); \
435+
} STMT_END
436+
#define mariadb_list_remove(list, entry) \
437+
STMT_START { \
438+
if ((entry)->prev) \
439+
(entry)->prev->next = (entry)->next; \
440+
if ((entry)->next) \
441+
(entry)->next->prev = (entry)->prev; \
442+
if ((list) == (entry)) \
443+
(list) = (entry)->next; \
444+
Safefree((entry)); \
445+
(entry) = NULL; \
446+
} STMT_END
447+
448+
420449
/*
421450
* This is our part of the driver handle. We receive the handle as
422451
* an "SV*", say "drh", and receive a pointer to the structure below
@@ -429,6 +458,9 @@ enum av_attribs {
429458
*/
430459
struct imp_drh_st {
431460
dbih_drc_t com; /* MUST be first element in structure */
461+
462+
struct mariadb_list_entry *active_imp_dbhs; /* List of imp_dbh structures with active MYSQL* */
463+
struct mariadb_list_entry *taken_pmysqls; /* List of active MYSQL* from take_imp_data() */
432464
unsigned long int instances;
433465
bool non_embedded_started;
434466
#if !defined(HAVE_EMBEDDED) && defined(HAVE_BROKEN_INIT)
@@ -437,7 +469,6 @@ struct imp_drh_st {
437469
bool embedded_started;
438470
SV *embedded_args;
439471
SV *embedded_groups;
440-
AV *taken_pmysqls; /* List of active MYSQL* structures from take_imp_data() */
441472
};
442473

443474

@@ -454,6 +485,7 @@ struct imp_drh_st {
454485
struct imp_dbh_st {
455486
dbih_dbc_t com; /* MUST be first element in structure */
456487

488+
struct mariadb_list_entry *list_entry; /* Entry of imp_drh->active_imp_dbhs list */
457489
MYSQL *pmysql;
458490
bool connected; /* Set to true after DBI->connect finished */
459491
bool auto_reconnect;

‎t/14destruct.t

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
use strict;
2+
use warnings;
3+
4+
use Test::More;
5+
use DBI;
6+
7+
use vars qw($test_dsn $test_user $test_password);
8+
use lib 't', '.';
9+
require 'lib.pl';
10+
11+
my $dbh = DbiTestConnect($test_dsn, $test_user, $test_password, { RaiseError => 1, PrintError => 0 });
12+
13+
plan tests => 3;
14+
15+
ok my $sth1 = $dbh->prepare("SELECT 1");
16+
ok my $sth2 = $dbh->prepare("SELECT 1", { mariadb_server_prepare => 1 });
17+
18+
# install a handler so that a warning about unfreed resources gets caught
19+
$SIG{__WARN__} = sub { die @_ };
20+
21+
END {
22+
my $sth1_copy = $sth1;
23+
my $sth2_copy = $sth2;
24+
pass;
25+
}

0 commit comments

Comments
 (0)