Skip to content

Commit 9e7da1b

Browse files
Ole John AskeDaniel Horecki
authored andcommitted
Bug#27675005 NDB::DROPEVENTOPERATION() CAUSED CRASH, - GCC CODE GEN. BUG?
When Ndb::DropEventOperation() decided to clean up any pending event, it failed to correctly clear the 'tail' pointer to the list of 'Gci_ops' being deleted and discarded. Thus the 'tail' pointer refers a deleted Gci_ops object. Later arriving Gci_ops may then be inserted as 'next' Gci_ops to this deleted object, causing memory corription and other havoc. Root cause is that we explicitely called the EventBufData_list d'tor from NdbEventBuffer::free_list() as a convinient way of releasing any Gci_op / Gci_ops in the 'list'. This d'tor also updated the head and tail pointer of the list as each contained element is deleted. However, with the (new) 'dead storage elimination'-optimization introduced in gcc 6.0, the updated 'head' and 'tail' in the destructed 'list' is not considered persistent after the destruction, thus the code is eliminated by the optimizer. This patch replace the explicit call to the d'tor by calling the new method ::delete_gci_ops(). This methods essentially does the same as the d'tor, which now is also changed to call this method. Not calling the d'tor, disalowes the compiler to do dse-optimization of the cleared head / tail pointer, which removes the root cause. Also introduce a Gci_ops d'tor which takes over delete[] of Gci_op contained within it. This replace part of the 'delete' code from delete_next_gci_ops() (cherry picked from commit f22bdc5f0be4145f352527c7b4124a6d434a4678)
1 parent 5eb510f commit 9e7da1b

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

storage/ndb/src/ndbapi/NdbEventOperationImpl.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
2+
Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License as published by
@@ -3974,7 +3974,7 @@ NdbEventBuffer::free_list(EventBufData_list &list)
39743974
list.m_head = list.m_tail = NULL;
39753975
list.m_count = list.m_sz = 0;
39763976
}
3977-
list.~EventBufData_list(); // free gci ops
3977+
list.delete_gci_ops(); // free gci ops
39783978
}
39793979

39803980
void EventBufData_list::append_list(EventBufData_list *list,

storage/ndb/src/ndbapi/NdbEventOperationImpl.hpp

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
2+
Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
33
44
This program is free software; you can redistribute it and/or modify
55
it under the terms of the GNU General Public License as published by
@@ -141,6 +141,8 @@ class EventBufData_list
141141
EventBufData_list();
142142
~EventBufData_list();
143143

144+
// delete all Gci_op[] / Gci_ops[] in this EventBufData_list
145+
void delete_gci_ops();
144146
// remove first and return its size
145147
void remove_first(Uint32 & full_count, Uint32 & full_sz);
146148
// for remove+append avoid double call to get_full_size()
@@ -199,7 +201,14 @@ class EventBufData_list
199201
m_next(NULL),
200202
m_gci_op_count(0)
201203
{};
202-
~Gci_ops() {};
204+
~Gci_ops()
205+
{
206+
if (m_gci_op_list)
207+
{
208+
DBUG_PRINT_EVENT("info", ("delete m_gci_op_list: %p", m_gci_op_list));
209+
delete [] m_gci_op_list;
210+
}
211+
};
203212

204213
MonotonicEpoch m_gci;
205214
Uint32 m_error;
@@ -238,7 +247,7 @@ EventBufData_list::EventBufData_list()
238247
m_count(0),
239248
m_sz(0),
240249
m_gci_op_list(NULL),
241-
m_gci_ops_list_tail(0),
250+
m_gci_ops_list_tail(NULL),
242251
m_gci_op_alloc(0)
243252
{
244253
DBUG_ENTER_EVENT("EventBufData_list::EventBufData_list");
@@ -249,7 +258,13 @@ EventBufData_list::EventBufData_list()
249258
inline
250259
EventBufData_list::~EventBufData_list()
251260
{
252-
DBUG_ENTER_EVENT("EventBufData_list::~EventBufData_list");
261+
delete_gci_ops();
262+
}
263+
264+
inline
265+
void EventBufData_list::delete_gci_ops()
266+
{
267+
DBUG_ENTER_EVENT("EventBufData_list::delete_gci_ops");
253268
DBUG_PRINT_EVENT("info", ("this: %p m_is_not_multi_list: %u",
254269
this, m_is_not_multi_list));
255270
if (m_is_not_multi_list)
@@ -342,12 +357,6 @@ EventBufData_list::delete_next_gci_ops()
342357
assert(!m_is_not_multi_list);
343358
Gci_ops *first = m_gci_ops_list;
344359
m_gci_ops_list = first->m_next;
345-
if (first->m_gci_op_list)
346-
{
347-
DBUG_PRINT_EVENT("info", ("this: %p delete m_gci_op_list: %p",
348-
this, first->m_gci_op_list));
349-
delete [] first->m_gci_op_list;
350-
}
351360
delete first;
352361
if (m_gci_ops_list == 0)
353362
m_gci_ops_list_tail = 0;

0 commit comments

Comments
 (0)