Skip to content

Commit 34e5001

Browse files
author
Marcelo Ferreira
committed
PT-SOMETHING - fixed connection leak due to confusion on parent flag
Introduces a new argument to Cxn to flag that connection should not be closed automatically with InactiveDestroy. The code had two different purposes to the same argument - replication topology parent and processes parent in fork scenarios leading to a leak.
1 parent 858e352 commit 34e5001

11 files changed

Lines changed: 230 additions & 39 deletions

‎bin/pt-archiver‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4742,6 +4742,7 @@ sub new {
47424742
DSNParser => $dp,
47434743
is_cluster_node => undef,
47444744
parent => $args{parent},
4745+
preserve_dbh => $args{preserve_dbh},
47454746
};
47464747

47474748
return bless $self, $class;
@@ -4800,7 +4801,7 @@ sub set_dbh {
48004801
$self->{hostname} = $hostname;
48014802
}
48024803

4803-
if ( $self->{parent} ) {
4804+
if ( $self->{preserve_dbh} ) {
48044805
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
48054806
$dbh->{InactiveDestroy} = 1;
48064807
}
@@ -4923,7 +4924,7 @@ sub DESTROY {
49234924

49244925
PTDEBUG && _d('Destroying cxn');
49254926

4926-
if ( $self->{parent} ) {
4927+
if ( $self->{preserve_dbh} ) {
49274928
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
49284929
}
49294930
elsif ( $self->{dbh}

‎bin/pt-config-diff‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,7 @@ sub new {
23682368
DSNParser => $dp,
23692369
is_cluster_node => undef,
23702370
parent => $args{parent},
2371+
preserve_dbh => $args{preserve_dbh},
23712372
};
23722373

23732374
return bless $self, $class;
@@ -2426,7 +2427,7 @@ sub set_dbh {
24262427
$self->{hostname} = $hostname;
24272428
}
24282429

2429-
if ( $self->{parent} ) {
2430+
if ( $self->{preserve_dbh} ) {
24302431
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
24312432
$dbh->{InactiveDestroy} = 1;
24322433
}
@@ -2549,7 +2550,7 @@ sub DESTROY {
25492550

25502551
PTDEBUG && _d('Destroying cxn');
25512552

2552-
if ( $self->{parent} ) {
2553+
if ( $self->{preserve_dbh} ) {
25532554
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
25542555
}
25552556
elsif ( $self->{dbh}

‎bin/pt-deadlock-logger‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2715,6 +2715,7 @@ sub new {
27152715
DSNParser => $dp,
27162716
is_cluster_node => undef,
27172717
parent => $args{parent},
2718+
preserve_dbh => $args{preserve_dbh},
27182719
};
27192720

27202721
return bless $self, $class;
@@ -2773,7 +2774,7 @@ sub set_dbh {
27732774
$self->{hostname} = $hostname;
27742775
}
27752776

2776-
if ( $self->{parent} ) {
2777+
if ( $self->{preserve_dbh} ) {
27772778
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
27782779
$dbh->{InactiveDestroy} = 1;
27792780
}
@@ -2896,7 +2897,7 @@ sub DESTROY {
28962897

28972898
PTDEBUG && _d('Destroying cxn');
28982899

2899-
if ( $self->{parent} ) {
2900+
if ( $self->{preserve_dbh} ) {
29002901
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
29012902
}
29022903
elsif ( $self->{dbh}
@@ -4711,7 +4712,7 @@ sub main {
47114712

47124713
my $src = Cxn->new(
47134714
dsn_string => shift @ARGV,
4714-
parent => $o->get('daemonize'),
4715+
preserve_dbh => $o->get('daemonize'),
47154716
DSNParser => $dp,
47164717
OptionParser => $o,
47174718
);
@@ -4735,7 +4736,7 @@ sub main {
47354736
$dst = Cxn->new(
47364737
dsn => $dst_dsn,
47374738
prev_dsn => ($src ? $src->dsn : undef),
4738-
parent => $o->get('daemonize'),
4739+
preserve_dbh => $o->get('daemonize'),
47394740
DSNParser => $dp,
47404741
OptionParser => $o,
47414742
set => $set_tz,
@@ -4806,13 +4807,13 @@ sub main {
48064807

48074808
# If we daemonized, the parent has already exited and we're the child.
48084809
# We shared a copy of every Cxn with the parent, and the parent's copies
4809-
# were destroyed but the dbhs were not disconnected because the parent
4810+
# were destroyed but the dbhs were not disconnected because the preserve_dbh
48104811
# attrib was true. Now, as the child, set it false so the dbhs will be
48114812
# disconnected when our Cxn copies are destroyed. If we didn't daemonize,
48124813
# then we're not really a parent (since we have no children), so set it
48134814
# false to auto-disconnect the dbhs when our Cxns are destroyed.
4814-
$src->{parent} = 0;
4815-
$dst->{parent} = 0 if $dst;
4815+
$src->{preserve_dbh} = 0;
4816+
$dst->{preserve_dbh} = 0 if $dst;
48164817

48174818
# ########################################################################
48184819
# Do the version-check

‎bin/pt-fk-error-logger‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1869,6 +1869,7 @@ sub new {
18691869
DSNParser => $dp,
18701870
is_cluster_node => undef,
18711871
parent => $args{parent},
1872+
preserve_dbh => $args{preserve_dbh},
18721873
};
18731874

18741875
return bless $self, $class;
@@ -1927,7 +1928,7 @@ sub set_dbh {
19271928
$self->{hostname} = $hostname;
19281929
}
19291930

1930-
if ( $self->{parent} ) {
1931+
if ( $self->{preserve_dbh} ) {
19311932
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
19321933
$dbh->{InactiveDestroy} = 1;
19331934
}
@@ -2050,7 +2051,7 @@ sub DESTROY {
20502051

20512052
PTDEBUG && _d('Destroying cxn');
20522053

2053-
if ( $self->{parent} ) {
2054+
if ( $self->{preserve_dbh} ) {
20542055
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
20552056
}
20562057
elsif ( $self->{dbh}
@@ -4177,7 +4178,7 @@ sub main {
41774178
if ( my $src_dsn_string = shift @ARGV ) {
41784179
$src = Cxn->new(
41794180
dsn_string => $src_dsn_string,
4180-
parent => $o->get('daemonize'),
4181+
preserve_dbh => $o->get('daemonize'),
41814182
DSNParser => $dp,
41824183
OptionParser => $o,
41834184
);
@@ -4188,7 +4189,7 @@ sub main {
41884189
$dst = Cxn->new(
41894190
dsn => $dst_dsn,
41904191
prev_dsn => ($src ? $src->dsn : undef),
4191-
parent => $o->get('daemonize'),
4192+
preserve_dbh => $o->get('daemonize'),
41924193
DSNParser => $dp,
41934194
OptionParser => $o,
41944195
);
@@ -4244,13 +4245,13 @@ sub main {
42444245

42454246
# If we daemonized, the parent has already exited and we're the child.
42464247
# We shared a copy of every Cxn with the parent, and the parent's copies
4247-
# were destroyed but the dbhs were not disconnected because the parent
4248+
# were destroyed but the dbhs were not disconnected because the preserve_dbh
42484249
# attrib was true. Now, as the child, set it false so the dbhs will be
42494250
# disconnected when our Cxn copies are destroyed. If we didn't daemonize,
42504251
# then we're not really a parent (since we have no children), so set it
42514252
# false to auto-disconnect the dbhs when our Cxns are destroyed.
4252-
$src->{parent} = 0;
4253-
$dst->{parent} = 0 if $dst;
4253+
$src->{preserve_dbh} = 0;
4254+
$dst->{preserve_dbh} = 0 if $dst;
42544255

42554256
# ########################################################################
42564257
# Do the version-check

‎bin/pt-kill‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5557,6 +5557,7 @@ sub new {
55575557
DSNParser => $dp,
55585558
is_cluster_node => undef,
55595559
parent => $args{parent},
5560+
preserve_dbh => $args{preserve_dbh},
55605561
};
55615562

55625563
return bless $self, $class;
@@ -5615,7 +5616,7 @@ sub set_dbh {
56155616
$self->{hostname} = $hostname;
56165617
}
56175618

5618-
if ( $self->{parent} ) {
5619+
if ( $self->{preserve_dbh} ) {
56195620
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
56205621
$dbh->{InactiveDestroy} = 1;
56215622
}
@@ -5738,7 +5739,7 @@ sub DESTROY {
57385739

57395740
PTDEBUG && _d('Destroying cxn');
57405741

5741-
if ( $self->{parent} ) {
5742+
if ( $self->{preserve_dbh} ) {
57425743
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
57435744
}
57445745
elsif ( $self->{dbh}
@@ -7245,7 +7246,7 @@ sub main {
72457246
$cxn = Cxn->new(
72467247
dsn_string => shift @ARGV,
72477248
NAME_lc => 0,
7248-
parent => $o->get('daemonize'),
7249+
preserve_dbh => $o->get('daemonize'),
72497250
DSNParser => $dp,
72507251
OptionParser => $o,
72517252
);
@@ -7444,12 +7445,12 @@ sub main {
74447445

74457446
# If we daemonized, the parent has already exited and we're the child.
74467447
# We shared a copy of every Cxn with the parent, and the parent's copies
7447-
# were destroyed but the dbhs were not disconnected because the parent
7448+
# were destroyed but the dbhs were not disconnected because the preserve_dbh
74487449
# attrib was true. Now, as the child, set it false so the dbhs will be
74497450
# disconnected when our Cxn copies are destroyed. If we didn't daemonize,
74507451
# then we're not really a parent (since we have no children), so set it
74517452
# false to auto-disconnect the dbhs when our Cxns are destroyed.
7452-
$cxn->{parent} = 0 if $cxn;
7453+
$cxn->{preserve_dbh} = 0 if $cxn;
74537454

74547455
# ########################################################################
74557456
# Do the version-check

‎bin/pt-online-schema-change‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3984,6 +3984,7 @@ sub new {
39843984
DSNParser => $dp,
39853985
is_cluster_node => undef,
39863986
parent => $args{parent},
3987+
preserve_dbh => $args{preserve_dbh},
39873988
};
39883989

39893990
return bless $self, $class;
@@ -4042,7 +4043,7 @@ sub set_dbh {
40424043
$self->{hostname} = $hostname;
40434044
}
40444045

4045-
if ( $self->{parent} ) {
4046+
if ( $self->{preserve_dbh} ) {
40464047
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
40474048
$dbh->{InactiveDestroy} = 1;
40484049
}
@@ -4165,7 +4166,7 @@ sub DESTROY {
41654166

41664167
PTDEBUG && _d('Destroying cxn');
41674168

4168-
if ( $self->{parent} ) {
4169+
if ( $self->{preserve_dbh} ) {
41694170
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
41704171
}
41714172
elsif ( $self->{dbh}

‎bin/pt-table-checksum‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3676,6 +3676,7 @@ sub new {
36763676
DSNParser => $dp,
36773677
is_cluster_node => undef,
36783678
parent => $args{parent},
3679+
preserve_dbh => $args{preserve_dbh},
36793680
};
36803681

36813682
return bless $self, $class;
@@ -3734,7 +3735,7 @@ sub set_dbh {
37343735
$self->{hostname} = $hostname;
37353736
}
37363737

3737-
if ( $self->{parent} ) {
3738+
if ( $self->{preserve_dbh} ) {
37383739
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
37393740
$dbh->{InactiveDestroy} = 1;
37403741
}
@@ -3857,7 +3858,7 @@ sub DESTROY {
38573858

38583859
PTDEBUG && _d('Destroying cxn');
38593860

3860-
if ( $self->{parent} ) {
3861+
if ( $self->{preserve_dbh} ) {
38613862
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
38623863
}
38633864
elsif ( $self->{dbh}

‎bin/pt-upgrade‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2542,6 +2542,7 @@ sub new {
25422542
DSNParser => $dp,
25432543
is_cluster_node => undef,
25442544
parent => $args{parent},
2545+
preserve_dbh => $args{preserve_dbh},
25452546
};
25462547

25472548
return bless $self, $class;
@@ -2600,7 +2601,7 @@ sub set_dbh {
26002601
$self->{hostname} = $hostname;
26012602
}
26022603

2603-
if ( $self->{parent} ) {
2604+
if ( $self->{preserve_dbh} ) {
26042605
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
26052606
$dbh->{InactiveDestroy} = 1;
26062607
}
@@ -2723,7 +2724,7 @@ sub DESTROY {
27232724

27242725
PTDEBUG && _d('Destroying cxn');
27252726

2726-
if ( $self->{parent} ) {
2727+
if ( $self->{preserve_dbh} ) {
27272728
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
27282729
}
27292730
elsif ( $self->{dbh}

‎lib/Cxn.pm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ sub new {
111111
DSNParser => $dp,
112112
is_cluster_node => undef,
113113
parent => $args{parent},
114+
preserve_dbh => $args{preserve_dbh},
114115
};
115116

116117
return bless $self, $class;
@@ -184,7 +185,7 @@ sub set_dbh {
184185
$self->{hostname} = $hostname;
185186
}
186187

187-
if ( $self->{parent} ) {
188+
if ( $self->{preserve_dbh} ) {
188189
PTDEBUG && _d($dbh, 'Setting InactiveDestroy=1 in parent');
189190
$dbh->{InactiveDestroy} = 1;
190191
}
@@ -334,7 +335,7 @@ sub DESTROY {
334335

335336
PTDEBUG && _d('Destroying cxn');
336337

337-
if ( $self->{parent} ) {
338+
if ( $self->{preserve_dbh} ) {
338339
PTDEBUG && _d($self->{dbh}, 'Not disconnecting dbh in parent');
339340
}
340341
elsif ( $self->{dbh}

0 commit comments

Comments
 (0)