Skip to content

Commit 9590409

Browse files
author
Andrei Elkin
committed
Bug#14152637 - SID_MAP GLOBAL INITIALIZATION IS OUT OF SEQUENCE, CAUSE CRASHES
Automatic initialization of GTID global objects before main() should not have been designed in the first place. Apparently it hurts PS and can easily lead to crashes. Besides, the GTID objects' mutex, cond-var:s and locks were not associated with PS keys. Fixed with the objects explicit initialization and implementing associations with PS keys.
1 parent 5b5f08b commit 9590409

23 files changed

Lines changed: 320 additions & 230 deletions

‎client/mysqlbinlog.cc‎

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,11 @@ static MYSQL* mysql = NULL;
153153
static char* dirname_for_local_load= 0;
154154
static uint opt_server_id_bits = 0;
155155
static ulong opt_server_id_mask = 0;
156+
Sid_map *global_sid_map= NULL;
157+
Checkable_rwlock *global_sid_lock= NULL;
158+
Gtid_set *gtid_set_included= NULL;
159+
Gtid_set *gtid_set_excluded= NULL;
160+
156161

157162
/**
158163
Pointer to the Format_description_log_event of the currently active binlog.
@@ -181,9 +186,6 @@ static char *opt_include_gtids_str= NULL,
181186
*opt_exclude_gtids_str= NULL;
182187
static my_bool opt_skip_gtids= 0;
183188
static bool filter_based_on_gtids= false;
184-
static Gtid_set gtid_set_included(&global_sid_map);
185-
static Gtid_set gtid_set_excluded(&global_sid_map);
186-
187189
static Exit_status dump_local_log_entries(PRINT_EVENT_INFO *print_event_info,
188190
const char* logname);
189191
static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
@@ -728,14 +730,14 @@ static bool shall_skip_gtids(Log_event* ev)
728730
if (opt_include_gtids_str != NULL)
729731
{
730732
filtered= filtered ||
731-
!gtid_set_included.contains_gtid(gtid->get_sidno(true),
733+
!gtid_set_included->contains_gtid(gtid->get_sidno(true),
732734
gtid->get_gno());
733735
}
734736

735737
if (opt_exclude_gtids_str != NULL)
736738
{
737739
filtered= filtered ||
738-
gtid_set_excluded.contains_gtid(gtid->get_sidno(true),
740+
gtid_set_excluded->contains_gtid(gtid->get_sidno(true),
739741
gtid->get_gno());
740742
}
741743
filter_based_on_gtids= filtered;
@@ -1915,10 +1917,10 @@ static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
19151917
{
19161918
command= COM_BINLOG_DUMP_GTID;
19171919

1918-
Gtid_set gtid_set(&global_sid_map);
1919-
global_sid_lock.rdlock();
1920+
Gtid_set gtid_set(global_sid_map);
1921+
global_sid_lock->rdlock();
19201922

1921-
gtid_set.add_gtid_set(&gtid_set_excluded);
1923+
gtid_set.add_gtid_set(gtid_set_excluded);
19221924

19231925
// allocate buffer
19241926
size_t unused_size= 0;
@@ -1931,7 +1933,7 @@ static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
19311933
if (!(command_buffer= (uchar *) my_malloc(allocation_size, MYF(MY_WME))))
19321934
{
19331935
error("Got fatal error allocating memory.");
1934-
global_sid_lock.unlock();
1936+
global_sid_lock->unlock();
19351937
DBUG_RETURN(ERROR_STOP);
19361938
}
19371939
uchar* ptr_buffer= command_buffer;
@@ -1966,7 +1968,7 @@ static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info,
19661968
unused_size= ::BINLOG_DATA_SIZE_INFO_SIZE + encoded_data_size;
19671969
}
19681970

1969-
global_sid_lock.unlock();
1971+
global_sid_lock->unlock();
19701972

19711973
command_size= ptr_buffer - command_buffer;
19721974
DBUG_ASSERT(command_size == (allocation_size - unused_size - 1));
@@ -2535,35 +2537,70 @@ static int args_post_process(void)
25352537
}
25362538
}
25372539

2538-
global_sid_lock.rdlock();
2540+
global_sid_lock->rdlock();
25392541

25402542
if (opt_include_gtids_str != NULL)
25412543
{
2542-
if (gtid_set_included.add_gtid_text(opt_include_gtids_str) !=
2544+
if (gtid_set_included->add_gtid_text(opt_include_gtids_str) !=
25432545
RETURN_STATUS_OK)
25442546
{
25452547
error("Could not configure --include-gtids '%s'", opt_include_gtids_str);
2546-
global_sid_lock.unlock();
2548+
global_sid_lock->unlock();
25472549
DBUG_RETURN(ERROR_STOP);
25482550
}
25492551
}
25502552

25512553
if (opt_exclude_gtids_str != NULL)
25522554
{
2553-
if (gtid_set_excluded.add_gtid_text(opt_exclude_gtids_str) !=
2555+
if (gtid_set_excluded->add_gtid_text(opt_exclude_gtids_str) !=
25542556
RETURN_STATUS_OK)
25552557
{
25562558
error("Could not configure --exclude-gtids '%s'", opt_exclude_gtids_str);
2557-
global_sid_lock.unlock();
2559+
global_sid_lock->unlock();
25582560
DBUG_RETURN(ERROR_STOP);
25592561
}
25602562
}
25612563

2562-
global_sid_lock.unlock();
2564+
global_sid_lock->unlock();
25632565

25642566
DBUG_RETURN(OK_CONTINUE);
25652567
}
25662568

2569+
/**
2570+
GTID cleanup destroys objects and reset their pointer.
2571+
Function is reentrant.
2572+
*/
2573+
inline void gtid_client_cleanup()
2574+
{
2575+
delete global_sid_lock;
2576+
delete global_sid_map;
2577+
delete gtid_set_excluded;
2578+
delete gtid_set_included;
2579+
global_sid_lock= NULL;
2580+
global_sid_map= NULL;
2581+
gtid_set_excluded= NULL;
2582+
gtid_set_included= NULL;
2583+
}
2584+
2585+
/**
2586+
GTID initialization.
2587+
2588+
@return true if allocation does not succeed
2589+
false if OK
2590+
*/
2591+
inline bool gtid_client_init()
2592+
{
2593+
bool res=
2594+
(!(global_sid_lock= new Checkable_rwlock) ||
2595+
!(global_sid_map= new Sid_map(global_sid_lock)) ||
2596+
!(gtid_set_excluded= new Gtid_set(global_sid_map)) ||
2597+
!(gtid_set_included= new Gtid_set(global_sid_map)));
2598+
if (res)
2599+
{
2600+
gtid_client_cleanup();
2601+
}
2602+
return res;
2603+
}
25672604

25682605
int main(int argc, char** argv)
25692606
{
@@ -2604,6 +2641,12 @@ int main(int argc, char** argv)
26042641
exit(1);
26052642
}
26062643

2644+
if (gtid_client_init())
2645+
{
2646+
error("Could not initialize GTID structuress.");
2647+
exit(1);
2648+
}
2649+
26072650
/* Check for argument conflicts and do any post-processing */
26082651
if (args_post_process() == ERROR_STOP)
26092652
exit(1);
@@ -2700,6 +2743,7 @@ int main(int argc, char** argv)
27002743
load_processor.destroy();
27012744
/* We cannot free DBUG, it is used in global destructors after exit(). */
27022745
my_end(my_end_arg | MY_DONT_FREE_DBUG);
2746+
gtid_client_cleanup();
27032747

27042748
exit(retval == ERROR_STOP ? 1 : 0);
27052749
/* Keep compilers happy. */

‎mysql-test/suite/perfschema/r/dml_setup_instruments.result‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ where name like 'Wait/Synch/Rwlock/sql/%'
1919
and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock')
2020
order by name limit 10;
2121
NAME ENABLED TIMED
22+
wait/synch/rwlock/sql/gtid_commit_rollback YES YES
2223
wait/synch/rwlock/sql/LOCK_dboptions YES YES
2324
wait/synch/rwlock/sql/LOCK_grant YES YES
2425
wait/synch/rwlock/sql/LOCK_system_variables_hash YES YES
@@ -28,7 +29,6 @@ wait/synch/rwlock/sql/LOGGER::LOCK_logger YES YES
2829
wait/synch/rwlock/sql/MDL_context::LOCK_waiting_for YES YES
2930
wait/synch/rwlock/sql/MDL_lock::rwlock YES YES
3031
wait/synch/rwlock/sql/Query_cache_query::lock YES YES
31-
wait/synch/rwlock/sql/THR_LOCK_servers YES YES
3232
select * from performance_schema.setup_instruments
3333
where name like 'Wait/Synch/Cond/sql/%'
3434
and name not in (

‎sql/binlog.cc‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -924,24 +924,24 @@ gtid_before_write_cache(THD* thd, binlog_cache_data* cache_data)
924924

925925
Group_cache* group_cache= &cache_data->group_cache;
926926

927-
global_sid_lock.rdlock();
927+
global_sid_lock->rdlock();
928928

929929
if (thd->variables.gtid_next.type == AUTOMATIC_GROUP)
930930
{
931931
if (group_cache->generate_automatic_gno(thd) !=
932932
RETURN_STATUS_OK)
933933
{
934-
global_sid_lock.unlock();
934+
global_sid_lock->unlock();
935935
DBUG_RETURN(1);
936936
}
937937
}
938938
if (write_empty_groups_to_cache(thd, cache_data) != 0)
939939
{
940-
global_sid_lock.unlock();
940+
global_sid_lock->unlock();
941941
DBUG_RETURN(1);
942942
}
943943

944-
global_sid_lock.unlock();
944+
global_sid_lock->unlock();
945945

946946
/*
947947
If an automatic group number was generated, change the first event
@@ -2534,14 +2534,14 @@ bool MYSQL_BIN_LOG::init_gtid_sets(Gtid_set *all_gtids, Gtid_set *lost_gtids,
25342534
if (all_gtids != NULL)
25352535
mysql_mutex_lock(&LOCK_log);
25362536
mysql_mutex_lock(&LOCK_index);
2537-
global_sid_lock.wrlock();
2537+
global_sid_lock->wrlock();
25382538
}
25392539
else
25402540
{
25412541
if (all_gtids != NULL)
25422542
mysql_mutex_assert_owner(&LOCK_log);
25432543
mysql_mutex_assert_owner(&LOCK_index);
2544-
global_sid_lock.assert_some_wrlock();
2544+
global_sid_lock->assert_some_wrlock();
25452545
}
25462546

25472547
// Gather the set of files to be accessed.
@@ -2627,7 +2627,7 @@ bool MYSQL_BIN_LOG::init_gtid_sets(Gtid_set *all_gtids, Gtid_set *lost_gtids,
26272627
lost_gtids->dbug_print("lost_gtids");
26282628
if (need_lock)
26292629
{
2630-
global_sid_lock.unlock();
2630+
global_sid_lock->unlock();
26312631
mysql_mutex_unlock(&LOCK_index);
26322632
if (all_gtids != NULL)
26332633
mysql_mutex_unlock(&LOCK_log);
@@ -2778,12 +2778,12 @@ bool MYSQL_BIN_LOG::open_binlog(const char *log_name,
27782778
if (current_thd && gtid_mode > 0)
27792779
{
27802780
if (need_sid_lock)
2781-
global_sid_lock.wrlock();
2781+
global_sid_lock->wrlock();
27822782
else
2783-
global_sid_lock.assert_some_wrlock();
2783+
global_sid_lock->assert_some_wrlock();
27842784
Previous_gtids_log_event prev_gtids_ev(previous_gtid_set);
27852785
if (need_sid_lock)
2786-
global_sid_lock.unlock();
2786+
global_sid_lock->unlock();
27872787
prev_gtids_ev.checksum_alg= s.checksum_alg;
27882788
if (prev_gtids_ev.write(&log_file))
27892789
goto err;
@@ -3275,7 +3275,7 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd)
32753275
mysql_mutex_lock(&LOCK_log);
32763276
mysql_mutex_lock(&LOCK_index);
32773277

3278-
global_sid_lock.wrlock();
3278+
global_sid_lock->wrlock();
32793279

32803280
/* Save variables so that we can reopen the log */
32813281
save_name=name;
@@ -3369,9 +3369,9 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd)
33693369
}
33703370
else
33713371
{
3372-
gtid_state.clear();
3372+
gtid_state->clear();
33733373
// don't clear global_sid_map because it's used by the relay log too
3374-
if (gtid_state.init() != 0)
3374+
if (gtid_state->init() != 0)
33753375
goto err;
33763376
}
33773377
#endif
@@ -3388,7 +3388,7 @@ bool MYSQL_BIN_LOG::reset_logs(THD* thd)
33883388
err:
33893389
if (error == 1)
33903390
name= const_cast<char*>(save_name);
3391-
global_sid_lock.unlock();
3391+
global_sid_lock->unlock();
33923392
mysql_mutex_unlock(&LOCK_thread_count);
33933393
mysql_mutex_unlock(&LOCK_index);
33943394
mysql_mutex_unlock(&LOCK_log);
@@ -3779,13 +3779,13 @@ int MYSQL_BIN_LOG::purge_logs(const char *to_log,
37793779
// Update gtid_state->lost_gtids
37803780
if (gtid_mode > 0 && !is_relay_log)
37813781
{
3782-
global_sid_lock.wrlock();
3782+
global_sid_lock->wrlock();
37833783
if (init_gtid_sets(NULL,
3784-
const_cast<Gtid_set *>(gtid_state.get_lost_gtids()),
3784+
const_cast<Gtid_set *>(gtid_state->get_lost_gtids()),
37853785
opt_master_verify_checksum,
37863786
false/*false=don't need lock*/))
37873787
goto err;
3788-
global_sid_lock.unlock();
3788+
global_sid_lock->unlock();
37893789
}
37903790

37913791
DBUG_EXECUTE_IF("crash_purge_critical_after_update_index", DBUG_SUICIDE(););
@@ -5280,13 +5280,13 @@ bool MYSQL_BIN_LOG::write_cache(THD *thd, binlog_cache_data *cache_data)
52805280
goto err;
52815281
}
52825282

5283-
global_sid_lock.rdlock();
5284-
if (gtid_state.update(thd, true) != RETURN_STATUS_OK)
5283+
global_sid_lock->rdlock();
5284+
if (gtid_state->update(thd, true) != RETURN_STATUS_OK)
52855285
{
5286-
global_sid_lock.unlock();
5286+
global_sid_lock->unlock();
52875287
goto err;
52885288
}
5289-
global_sid_lock.unlock();
5289+
global_sid_lock->unlock();
52905290
}
52915291
update_thd_next_event_pos(thd);
52925292
}

‎sql/binlog.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
432432
Previous_gtids_log_event.
433433
@param verify_checksum If true, checksums will be checked.
434434
@param need_lock If true, LOCK_log, LOCK_index, and
435-
global_sid_lock.wrlock are acquired; otherwise they are asserted
435+
global_sid_lock->wrlock are acquired; otherwise they are asserted
436436
to be taken already.
437437
@return false on success, true on error.
438438
*/

‎sql/log_event.cc‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12847,9 +12847,9 @@ Gtid_log_event::Gtid_log_event(THD* thd_arg, bool using_trans,
1284712847
spec= spec_arg ? *spec_arg : thd_arg->variables.gtid_next;
1284812848
if (spec.type == GTID_GROUP)
1284912849
{
12850-
global_sid_lock.rdlock();
12851-
sid= global_sid_map.sidno_to_sid(spec.gtid.sidno);
12852-
global_sid_lock.unlock();
12850+
global_sid_lock->rdlock();
12851+
sid= global_sid_map->sidno_to_sid(spec.gtid.sidno);
12852+
global_sid_lock->unlock();
1285312853
}
1285412854
else
1285512855
sid.clear();
@@ -12995,7 +12995,7 @@ Previous_gtids_log_event::Previous_gtids_log_event(const Gtid_set *set)
1299512995
Log_event::EVENT_IMMEDIATE_LOGGING)
1299612996
{
1299712997
DBUG_ENTER("Previous_gtids_log_event::Previous_gtids_log_event(THD *, const Gtid_set *)");
12998-
global_sid_lock.assert_some_lock();
12998+
global_sid_lock->assert_some_lock();
1299912999
buf_size= set->get_encoded_length();
1300013000
uchar *buffer= (uchar *) my_malloc(buf_size, MYF(MY_WME));
1300113001
if (buffer != NULL)
@@ -13013,9 +13013,9 @@ Previous_gtids_log_event::Previous_gtids_log_event(const Gtid_set *set)
1301313013
int Previous_gtids_log_event::pack_info(Protocol *protocol)
1301413014
{
1301513015
size_t length= 0;
13016-
global_sid_lock.rdlock();
13016+
global_sid_lock->rdlock();
1301713017
char *str= get_str(&length, &Gtid_set::default_string_format);
13018-
global_sid_lock.unlock();
13018+
global_sid_lock->unlock();
1301913019
if (str == NULL)
1302013020
return 1;
1302113021
protocol->store(str, length, &my_charset_bin);
@@ -13030,9 +13030,9 @@ void Previous_gtids_log_event::print(FILE *file,
1303013030
{
1303113031
IO_CACHE *const head= &print_event_info->head_cache;
1303213032

13033-
global_sid_lock.rdlock();
13033+
global_sid_lock->rdlock();
1303413034
char *str= get_str(NULL, &Gtid_set::commented_string_format);
13035-
global_sid_lock.unlock();
13035+
global_sid_lock->unlock();
1303613036
if (str != NULL)
1303713037
{
1303813038
if (!print_event_info->short_form)

‎sql/log_event.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4806,12 +4806,12 @@ class Gtid_log_event : public Log_event
48064806
if (spec.gtid.sidno < 0)
48074807
{
48084808
if (need_lock)
4809-
global_sid_lock.rdlock();
4809+
global_sid_lock->rdlock();
48104810
else
4811-
global_sid_lock.assert_some_lock();
4812-
spec.gtid.sidno= global_sid_map.add_sid(sid);
4811+
global_sid_lock->assert_some_lock();
4812+
spec.gtid.sidno= global_sid_map->add_sid(sid);
48134813
if (need_lock)
4814-
global_sid_lock.unlock();
4814+
global_sid_lock->unlock();
48154815
}
48164816
return spec.gtid.sidno;
48174817
}

0 commit comments

Comments
 (0)