Skip to content

Commit e25a8ff

Browse files
author
Steinar H. Gunderson
committed
Bug #31947917: WL#14071: ASAN LEAKS SEEN DURING SHUTDOWN
The list of input tables in filesort is currently represented by Prealloced_array<TABLE *, 4>. However, when going over the limit (ie., when grouping a join of five or more tables), it starts allocating memory from the heap, _not_ the MEM_ROOT. This causes memory leaks, since Filesort objects are not always destroyed. Change the table list to Mem_root_array<TABLE *> instead. Change-Id: I0cf046194f7a78816b12e77e169e00f52b358f1d
1 parent caa8f24 commit e25a8ff

11 files changed

+60
-58
lines changed

‎sql/basic_row_iterators.h

+9-11
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
#include "my_alloc.h"
3939
#include "my_base.h"
4040
#include "my_inttypes.h"
41-
#include "prealloced_array.h"
4241
#include "sql/mem_root_array.h"
4342
#include "sql/row_iterator.h"
4443
#include "sql/sql_list.h"
@@ -171,7 +170,7 @@ class SortBufferIterator final : public RowIterator {
171170
public:
172171
// "examined_rows", if not nullptr, is incremented for each successful Read().
173172
// The table is used solely for NULL row flags.
174-
SortBufferIterator(THD *thd, Prealloced_array<TABLE *, 4> tables,
173+
SortBufferIterator(THD *thd, Mem_root_array<TABLE *> tables,
175174
Filesort_info *sort, Sort_result *sort_result,
176175
ha_rows *examined_rows);
177176
~SortBufferIterator() override;
@@ -187,7 +186,7 @@ class SortBufferIterator final : public RowIterator {
187186
Sort_result *const m_sort_result;
188187
unsigned m_unpack_counter;
189188
ha_rows *const m_examined_rows;
190-
Prealloced_array<TABLE *, 4> m_tables;
189+
Mem_root_array<TABLE *> m_tables;
191190
};
192191

193192
/**
@@ -209,7 +208,7 @@ class SortBufferIndirectIterator final : public RowIterator {
209208
// The pushed condition can be nullptr.
210209
//
211210
// "examined_rows", if not nullptr, is incremented for each successful Read().
212-
SortBufferIndirectIterator(THD *thd, Prealloced_array<TABLE *, 4> tables,
211+
SortBufferIndirectIterator(THD *thd, Mem_root_array<TABLE *> tables,
213212
Sort_result *sort_result,
214213
bool ignore_not_found_rows, bool has_null_flags,
215214
ha_rows *examined_rows);
@@ -221,7 +220,7 @@ class SortBufferIndirectIterator final : public RowIterator {
221220

222221
private:
223222
Sort_result *const m_sort_result;
224-
Prealloced_array<TABLE *, 4> m_tables;
223+
Mem_root_array<TABLE *> m_tables;
225224
uint m_sum_ref_length;
226225
ha_rows *const m_examined_rows;
227226
uchar *m_cache_pos = nullptr, *m_cache_end = nullptr;
@@ -242,9 +241,8 @@ class SortFileIterator final : public RowIterator {
242241
public:
243242
// Takes ownership of tempfile.
244243
// The table is used solely for NULL row flags.
245-
SortFileIterator(THD *thd, Prealloced_array<TABLE *, 4> tables,
246-
IO_CACHE *tempfile, Filesort_info *sort,
247-
ha_rows *examined_rows);
244+
SortFileIterator(THD *thd, Mem_root_array<TABLE *> tables, IO_CACHE *tempfile,
245+
Filesort_info *sort, ha_rows *examined_rows);
248246
~SortFileIterator() override;
249247

250248
bool Init() override { return false; }
@@ -255,7 +253,7 @@ class SortFileIterator final : public RowIterator {
255253
private:
256254
uchar *const m_rec_buf;
257255
const uint m_buf_length;
258-
Prealloced_array<TABLE *, 4> m_tables;
256+
Mem_root_array<TABLE *> m_tables;
259257
IO_CACHE *const m_io_cache;
260258
Filesort_info *const m_sort;
261259
ha_rows *const m_examined_rows;
@@ -277,7 +275,7 @@ class SortFileIndirectIterator final : public RowIterator {
277275
// The pushed condition can be nullptr.
278276
//
279277
// "examined_rows", if not nullptr, is incremented for each successful Read().
280-
SortFileIndirectIterator(THD *thd, Prealloced_array<TABLE *, 4> tables,
278+
SortFileIndirectIterator(THD *thd, Mem_root_array<TABLE *> tables,
281279
IO_CACHE *tempfile, bool ignore_not_found_rows,
282280
bool has_null_flags, ha_rows *examined_rows);
283281
~SortFileIndirectIterator() override;
@@ -290,7 +288,7 @@ class SortFileIndirectIterator final : public RowIterator {
290288
private:
291289
IO_CACHE *m_io_cache = nullptr;
292290
ha_rows *const m_examined_rows;
293-
Prealloced_array<TABLE *, 4> m_tables;
291+
Mem_root_array<TABLE *> m_tables;
294292
uchar *m_ref_pos = nullptr;
295293
bool m_ignore_not_found_rows;
296294
bool m_has_null_flags;

‎sql/filesort.cc

+7-7
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ struct Mem_compare_queue_key {
142142
/* functions defined in this file */
143143

144144
static ha_rows read_all_rows(
145-
THD *thd, Sort_param *param, const Prealloced_array<TABLE *, 4> &tables,
145+
THD *thd, Sort_param *param, const Mem_root_array<TABLE *> &tables,
146146
table_map tables_to_get_rowid_for, Filesort_info *fs_info,
147147
IO_CACHE *chunk_file, IO_CACHE *tempfile,
148148
Bounded_queue<uchar *, uchar *, Sort_param, Mem_compare_queue_key> *pq,
@@ -161,7 +161,7 @@ static bool check_if_pq_applicable(Opt_trace_context *trace, Sort_param *param,
161161
ulong memory_available);
162162

163163
void Sort_param::decide_addon_fields(Filesort *file_sort,
164-
const Prealloced_array<TABLE *, 4> &tables,
164+
const Mem_root_array<TABLE *> &tables,
165165
bool force_sort_positions) {
166166
if (m_addon_fields_status != Addon_fields_status::unknown_status) {
167167
// Already decided.
@@ -223,7 +223,7 @@ void Sort_param::clear_addon_fields() {
223223
void Sort_param::init_for_filesort(Filesort *file_sort,
224224
Bounds_checked_array<st_sort_field> sf_array,
225225
uint sortlen,
226-
const Prealloced_array<TABLE *, 4> &tables,
226+
const Mem_root_array<TABLE *> &tables,
227227
ha_rows maxrows, bool remove_duplicates) {
228228
m_fixed_sort_length = sortlen;
229229
m_force_stable_sort = file_sort->m_force_stable_sort;
@@ -677,7 +677,7 @@ void filesort_free_buffers(TABLE *table, bool full) {
677677
}
678678
}
679679

680-
Filesort::Filesort(THD *thd, Prealloced_array<TABLE *, 4> tables_arg,
680+
Filesort::Filesort(THD *thd, Mem_root_array<TABLE *> tables_arg,
681681
bool keep_buffers_arg, ORDER *order, ha_rows limit_arg,
682682
bool force_stable_sort, bool remove_duplicates,
683683
bool sort_positions, bool unwrap_rollup)
@@ -856,7 +856,7 @@ class Filesort_error_handler : public Internal_error_handler {
856856
};
857857

858858
static bool alloc_and_make_sortkey(Sort_param *param, Filesort_info *fs_info,
859-
const Prealloced_array<TABLE *, 4> &tables,
859+
const Mem_root_array<TABLE *> &tables,
860860
size_t *key_length, size_t *longest_addons) {
861861
size_t min_bytes = 1;
862862
for (;;) { // Termination condition within loop.
@@ -942,7 +942,7 @@ static bool alloc_and_make_sortkey(Sort_param *param, Filesort_info *fs_info,
942942
*/
943943

944944
static ha_rows read_all_rows(
945-
THD *thd, Sort_param *param, const Prealloced_array<TABLE *, 4> &tables,
945+
THD *thd, Sort_param *param, const Mem_root_array<TABLE *> &tables,
946946
table_map tables_to_get_rowid_for, Filesort_info *fs_info,
947947
IO_CACHE *chunk_file, IO_CACHE *tempfile,
948948
Bounded_queue<uchar *, uchar *, Sort_param, Mem_compare_queue_key> *pq,
@@ -1388,7 +1388,7 @@ size_t make_sortkey_from_item(Item *item, Item_result result_type,
13881388
} // namespace
13891389

13901390
uint Sort_param::make_sortkey(Bounds_checked_array<uchar> dst,
1391-
const Prealloced_array<TABLE *, 4> &tables,
1391+
const Mem_root_array<TABLE *> &tables,
13921392
size_t *longest_addon_so_far) {
13931393
uchar *to = dst.array();
13941394
uchar *to_end = dst.array() + dst.size();

‎sql/filesort.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@
2626
#include <stddef.h>
2727
#include <sys/types.h>
2828

29+
#include "mem_root_array.h"
2930
#include "my_base.h" /* ha_rows */
3031
#include "my_dbug.h"
3132
#include "my_inttypes.h"
3233
#include "my_table_map.h"
33-
#include "prealloced_array.h"
3434
#include "sql/sort_param.h"
3535

3636
class Addon_fields;
@@ -52,7 +52,7 @@ class Filesort {
5252
public:
5353
THD *m_thd;
5454
/// The tables we are sorting.
55-
Prealloced_array<TABLE *, 4> tables;
55+
Mem_root_array<TABLE *> tables;
5656
/// If true, do not free the filesort buffers (use if you expect to sort many
5757
/// times, like in an uncacheable subquery).
5858
const bool keep_buffers;
@@ -75,7 +75,7 @@ class Filesort {
7575
// TODO(sgunders): Change tables to a table_map; however, currently
7676
// some semijoin tables are missing from select_lex->leaf_tables,
7777
// so we can't do that yet.
78-
Filesort(THD *thd, Prealloced_array<TABLE *, 4> tables, bool keep_buffers,
78+
Filesort(THD *thd, Mem_root_array<TABLE *> tables, bool keep_buffers,
7979
ORDER *order, ha_rows limit_arg, bool force_stable_sort,
8080
bool remove_duplicates, bool force_sort_positions,
8181
bool unwrap_rollup);

‎sql/join_optimizer/join_optimizer.cc

+4-5
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,8 @@ void CostingReceiver::ApplyDelayedPredicatesAfterJoin(
454454
/**
455455
Create a table array from a table bitmap.
456456
*/
457-
Prealloced_array<TABLE *, 4> CollectTables(SELECT_LEX *select_lex,
458-
table_map tmap) {
459-
Prealloced_array<TABLE *, 4> tables(PSI_NOT_INSTRUMENTED);
457+
Mem_root_array<TABLE *> CollectTables(SELECT_LEX *select_lex, table_map tmap) {
458+
Mem_root_array<TABLE *> tables(select_lex->join->thd->mem_root);
460459
for (TABLE_LIST *tl = select_lex->leaf_tables; tl != nullptr;
461460
tl = tl->next_leaf) {
462461
if (tl->map() & tmap) {
@@ -725,7 +724,7 @@ AccessPath *FindBestQueryPlan(THD *thd, SELECT_LEX *select_lex, string *trace) {
725724
return nullptr;
726725

727726
if (select_lex->is_explicitly_grouped()) {
728-
Prealloced_array<TABLE *, 4> tables =
727+
Mem_root_array<TABLE *> tables =
729728
CollectTables(select_lex, GetUsedTables(root_path));
730729
Filesort *filesort = new (thd->mem_root)
731730
Filesort(thd, std::move(tables), /*keep_buffers=*/false,
@@ -797,7 +796,7 @@ AccessPath *FindBestQueryPlan(THD *thd, SELECT_LEX *select_lex, string *trace) {
797796

798797
// Apply ORDER BY, if applicable.
799798
if (select_lex->is_ordered()) {
800-
Prealloced_array<TABLE *, 4> tables =
799+
Mem_root_array<TABLE *> tables =
801800
CollectTables(select_lex, GetUsedTables(root_path));
802801
Temp_table_param *temp_table_param = nullptr;
803802
TABLE *temp_table = nullptr;

‎sql/mem_root_array.h

+4
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
#include "my_alloc.h"
3131
#include "my_dbug.h"
32+
#include "sql/thr_malloc.h"
3233

3334
/**
3435
A typesafe replacement for DYNAMIC_ARRAY.
@@ -473,6 +474,9 @@ class Mem_root_array : public Mem_root_array_YY<Element_type> {
473474
Mem_root_array(MEM_ROOT *root, const Mem_root_array &x)
474475
: Mem_root_array(root, x.cbegin(), x.cend()) {}
475476

477+
Mem_root_array(std::initializer_list<Element_type> elements)
478+
: Mem_root_array(*THR_MALLOC, begin(elements), end(elements)) {}
479+
476480
~Mem_root_array() { super::clear(); }
477481

478482
// Not (yet) implemented.

‎sql/records.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ unique_ptr_destroy_only<RowIterator> init_table_iterator(
190190
my_b_inited(table->unique_result.io_cache)) {
191191
DBUG_PRINT("info", ("using SortFileIndirectIterator"));
192192
iterator = NewIterator<SortFileIndirectIterator>(
193-
thd, Prealloced_array<TABLE *, 4>{table}, table->unique_result.io_cache,
193+
thd, Mem_root_array<TABLE *>{table}, table->unique_result.io_cache,
194194
ignore_not_found_rows, /*has_null_flags=*/false,
195195
/*examined_rows=*/nullptr);
196196
table->unique_result.io_cache =
@@ -203,7 +203,7 @@ unique_ptr_destroy_only<RowIterator> init_table_iterator(
203203
DBUG_ASSERT(!table->unique_result.sorted_result_in_fsbuf);
204204
DBUG_PRINT("info", ("using SortBufferIndirectIterator (unique)"));
205205
iterator = NewIterator<SortBufferIndirectIterator>(
206-
thd, Prealloced_array<TABLE *, 4>{table}, &table->unique_result,
206+
thd, Mem_root_array<TABLE *>{table}, &table->unique_result,
207207
ignore_not_found_rows, /*has_null_flags=*/false,
208208
/*examined_rows=*/nullptr);
209209
} else {

‎sql/sort_param.h

+5-6
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
#include "my_inttypes.h"
3333
#include "my_io.h" // mysql_com.h needs my_socket
3434
#include "mysql_com.h" // Item_result
35-
#include "prealloced_array.h"
35+
#include "sql/mem_root_array.h"
3636
#include "sql/sql_array.h" // Bounds_checked_array
3737
#include "sql/sql_const.h"
3838
#include "sql/sql_sort.h" // Filesort_info
@@ -325,7 +325,7 @@ class Sort_param {
325325
/// make_sortkey() check the read set at runtime, at the cost of slightly less
326326
/// precise estimation of packed row size.
327327
void decide_addon_fields(Filesort *file_sort,
328-
const Prealloced_array<TABLE *, 4> &tables,
328+
const Mem_root_array<TABLE *> &tables,
329329
bool sort_positions);
330330

331331
/// Reset the decision made in decide_addon_fields(). Only used in exceptional
@@ -345,8 +345,7 @@ class Sort_param {
345345
*/
346346
void init_for_filesort(Filesort *file_sort,
347347
Bounds_checked_array<st_sort_field> sf_array,
348-
uint sortlen,
349-
const Prealloced_array<TABLE *, 4> &tables,
348+
uint sortlen, const Mem_root_array<TABLE *> &tables,
350349
ha_rows maxrows, bool remove_duplicates);
351350

352351
/**
@@ -389,12 +388,12 @@ class Sort_param {
389388
provably fit within the destination buffer.
390389
*/
391390
uint make_sortkey(Bounds_checked_array<uchar> dst,
392-
const Prealloced_array<TABLE *, 4> &tables,
391+
const Mem_root_array<TABLE *> &tables,
393392
size_t *longest_addons);
394393

395394
// Adapter for Bounded_queue.
396395
uint make_sortkey(uchar *dst, size_t dst_len,
397-
const Prealloced_array<TABLE *, 4> &tables) {
396+
const Mem_root_array<TABLE *> &tables) {
398397
size_t longest_addons = 0; // Unused.
399398
return make_sortkey(Bounds_checked_array<uchar>(dst, dst_len), tables,
400399
&longest_addons);

‎sql/sorting_iterator.cc

+22-21
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ using std::string;
6666
using std::vector;
6767

6868
SortFileIndirectIterator::SortFileIndirectIterator(
69-
THD *thd, Prealloced_array<TABLE *, 4> tables, IO_CACHE *tempfile,
69+
THD *thd, Mem_root_array<TABLE *> tables, IO_CACHE *tempfile,
7070
bool ignore_not_found_rows, bool has_null_flags, ha_rows *examined_rows)
7171
: RowIterator(thd),
7272
m_io_cache(tempfile),
@@ -185,7 +185,7 @@ int SortFileIndirectIterator::Read() {
185185

186186
template <bool Packed_addon_fields>
187187
SortFileIterator<Packed_addon_fields>::SortFileIterator(
188-
THD *thd, Prealloced_array<TABLE *, 4> tables, IO_CACHE *tempfile,
188+
THD *thd, Mem_root_array<TABLE *> tables, IO_CACHE *tempfile,
189189
Filesort_info *sort, ha_rows *examined_rows)
190190
: RowIterator(thd),
191191
m_rec_buf(sort->addon_fields->get_addon_buf()),
@@ -256,7 +256,7 @@ void SortFileIterator<Packed_addon_fields>::SetNullRowFlag(bool is_null_row) {
256256

257257
template <bool Packed_addon_fields>
258258
SortBufferIterator<Packed_addon_fields>::SortBufferIterator(
259-
THD *thd, Prealloced_array<TABLE *, 4> tables, Filesort_info *sort,
259+
THD *thd, Mem_root_array<TABLE *> tables, Filesort_info *sort,
260260
Sort_result *sort_result, ha_rows *examined_rows)
261261
: RowIterator(thd),
262262
m_sort(sort),
@@ -321,7 +321,7 @@ void SortBufferIterator<Packed_addon_fields>::SetNullRowFlag(bool is_null_row) {
321321
}
322322

323323
SortBufferIndirectIterator::SortBufferIndirectIterator(
324-
THD *thd, Prealloced_array<TABLE *, 4> tables, Sort_result *sort_result,
324+
THD *thd, Mem_root_array<TABLE *> tables, Sort_result *sort_result,
325325
bool ignore_not_found_rows, bool has_null_flags, ha_rows *examined_rows)
326326
: RowIterator(thd),
327327
m_sort_result(sort_result),
@@ -469,28 +469,29 @@ bool SortingIterator::Init() {
469469

470470
// Prepare the result iterator for actually reading the data. Read()
471471
// will proxy to it.
472-
const Prealloced_array<TABLE *, 4> &tables = m_filesort->tables;
472+
Mem_root_array<TABLE *> tables(thd()->mem_root, m_filesort->tables);
473473
if (m_sort_result.io_cache && my_b_inited(m_sort_result.io_cache)) {
474474
// Test if ref-records was used
475475
if (m_fs_info.using_addon_fields()) {
476476
DBUG_PRINT("info", ("using SortFileIterator"));
477477
if (m_fs_info.addon_fields->using_packed_addons())
478478
m_result_iterator.reset(
479479
new (&m_result_iterator_holder.sort_file_packed_addons)
480-
SortFileIterator<true>(thd(), tables, m_sort_result.io_cache,
481-
&m_fs_info, m_examined_rows));
480+
SortFileIterator<true>(thd(), std::move(tables),
481+
m_sort_result.io_cache, &m_fs_info,
482+
m_examined_rows));
482483
else
483484
m_result_iterator.reset(
484-
new (&m_result_iterator_holder.sort_file)
485-
SortFileIterator<false>(thd(), tables, m_sort_result.io_cache,
486-
&m_fs_info, m_examined_rows));
485+
new (&m_result_iterator_holder.sort_file) SortFileIterator<false>(
486+
thd(), std::move(tables), m_sort_result.io_cache, &m_fs_info,
487+
m_examined_rows));
487488
} else {
488489
m_result_iterator.reset(
489490
new (&m_result_iterator_holder.sort_file_indirect)
490-
SortFileIndirectIterator(thd(), tables, m_sort_result.io_cache,
491-
/*ignore_not_found_rows=*/false,
492-
/*has_null_flags=*/true,
493-
m_examined_rows));
491+
SortFileIndirectIterator(
492+
thd(), std::move(tables), m_sort_result.io_cache,
493+
/*ignore_not_found_rows=*/false,
494+
/*has_null_flags=*/true, m_examined_rows));
494495
}
495496
m_sort_result.io_cache =
496497
nullptr; // The result iterator has taken ownership.
@@ -502,21 +503,21 @@ bool SortingIterator::Init() {
502503
if (m_fs_info.addon_fields->using_packed_addons())
503504
m_result_iterator.reset(
504505
new (&m_result_iterator_holder.sort_buffer_packed_addons)
505-
SortBufferIterator<true>(thd(), tables, &m_fs_info,
506+
SortBufferIterator<true>(thd(), std::move(tables), &m_fs_info,
506507
&m_sort_result, m_examined_rows));
507508
else
508509
m_result_iterator.reset(
509510
new (&m_result_iterator_holder.sort_buffer)
510-
SortBufferIterator<false>(thd(), tables, &m_fs_info,
511+
SortBufferIterator<false>(thd(), std::move(tables), &m_fs_info,
511512
&m_sort_result, m_examined_rows));
512513
} else {
513514
DBUG_PRINT("info", ("using SortBufferIndirectIterator (sort)"));
514515
m_result_iterator.reset(
515516
new (&m_result_iterator_holder.sort_buffer_indirect)
516-
SortBufferIndirectIterator(thd(), tables, &m_sort_result,
517-
/*ignore_not_found_rows=*/false,
518-
/*has_null_flags=*/true,
519-
m_examined_rows));
517+
SortBufferIndirectIterator(
518+
thd(), std::move(tables), &m_sort_result,
519+
/*ignore_not_found_rows=*/false,
520+
/*has_null_flags=*/true, m_examined_rows));
520521
}
521522
}
522523

@@ -552,7 +553,7 @@ int SortingIterator::DoSort() {
552553

553554
template <bool Packed_addon_fields>
554555
inline void Filesort_info::unpack_addon_fields(
555-
const Prealloced_array<TABLE *, 4> &tables, uchar *buff) {
556+
const Mem_root_array<TABLE *> &tables, uchar *buff) {
556557
const uchar *nulls = buff + addon_fields->skip_bytes();
557558

558559
// Unpack table NULL flags.

0 commit comments

Comments
 (0)