Skip to content

Commit

Permalink
BUG#31606266 - SERVER HUNG WHILE CONFIGURING AFTER INSTALLING POSTFIX
Browse files Browse the repository at this point in the history
Description
===========
There is a race-condition between `event_base_loopbreak` and `event_base_loop`,
when `loopbreak` is called before other thread enters `base_loop`, is such case
all `breaks` and `exits` are ignored.
The break function was not designed to be called from different thread.

Fix
===
1. Enable multithreading in `libevent`.
2. Switch from `event_base_loopbreak`  to `event_base_loopexit`,
   where the last function, may be called from different thread.

After enabling multithreading in libevent, the accepting speed (doc 19) decreases
with following numbers:

| Name                                       | Baseline 20 | Document 19 | Change %|
| Description                                | time[us]    | time[us]    |         |
|============================================|=============|=============|=========|
| conn_plain<xclient_socket_nossl>/threads:1 | 425.3       | 453.9       | -6.3%   |
| conn_plain<xclient_socket_nossl>/threads:8 | 145.6       | 152.2       | -4.3%   |
| conn_plain<xclient_socket_nossl>/threads:16| 145.6       | 146.7       | -0.7%   |
| conn_plain<xclient_socket_nossl>/threads:32| 150.8       | 150.6       | 0.1%    |

RB: 24810
Reviewed by: Grzegorz Szwarc <grzegorz.szwarc@oracle.com>
Reviewed-by: Tor Didriksen <tor.didriksen@oracle.com>
  • Loading branch information
lkotula committed Jul 21, 2020
1 parent b49d739 commit 2b84f0e
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 4 deletions.
7 changes: 6 additions & 1 deletion cmake/libevent.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,15 @@ MACRO (FIND_SYSTEM_LIBEVENT)

FIND_LIBRARY(LIBEVENT_CORE event_core PATHS ${LIBEVENT_LIB_PATHS})
FIND_LIBRARY(LIBEVENT_EXTRA event_extra PATHS ${LIBEVENT_LIB_PATHS})
FIND_LIBRARY(LIBEVENT_PTHREADS event_pthreads PATHS ${LIBEVENT_LIB_PATHS})

## libevent_openssl.so is split out on Linux distros
FIND_LIBRARY(LIBEVENT_OPENSSL event_openssl PATHS ${LIBEVENT_LIB_PATHS})

IF (LIBEVENT_CORE AND LIBEVENT_INCLUDE_DIRECTORY)
SET(LIBEVENT_FOUND TRUE)
SET(LIBEVENT_LIBRARIES
${LIBEVENT_CORE} ${LIBEVENT_EXTRA} ${LIBEVENT_OPENSSL})
${LIBEVENT_CORE} ${LIBEVENT_EXTRA} ${LIBEVENT_OPENSSL} ${LIBEVENT_PTHREADS})
SET(LIBEVENT_INCLUDE_DIRS ${LIBEVENT_INCLUDE_DIRECTORY})
IF(NOT LIBEVENT_INCLUDE_DIRECTORY STREQUAL "/usr/include")
MESSAGE(STATUS "LIBEVENT_INCLUDE_DIRS ${LIBEVENT_INCLUDE_DIRS}")
Expand All @@ -98,6 +99,10 @@ ENDMACRO()

MACRO (MYSQL_USE_BUNDLED_LIBEVENT)
SET(LIBEVENT_LIBRARIES event_core event_extra event_openssl)
IF(CMAKE_USE_PTHREADS_INIT)
LIST(APPEND LIBEVENT_LIBRARIES event_pthreads)
ENDIF(CMAKE_USE_PTHREADS_INIT)

SET(LIBEVENT_BUNDLE_PATH "extra/libevent/libevent-2.1.11-stable")
SET(LIBEVENT_INCLUDE_DIRS
"${CMAKE_SOURCE_DIR}/${LIBEVENT_BUNDLE_PATH}/include"
Expand Down
25 changes: 22 additions & 3 deletions plugin/x/ngs/src/socket_events.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include <algorithm>
#include <cstdint>
#include <mutex>
#include <new>

#include "my_io.h" // NOLINT(build/include_subdir)
Expand All @@ -38,12 +39,25 @@
#ifdef WIN32
#pragma warning(push)
#pragma warning(disable : 4005)
#endif // WIN32
#include <event.h> // libevent
#endif // WIN32
#include <event2/event.h> // libevent
#include <event2/event_compat.h>
#include <event2/event_struct.h>
#include <event2/thread.h>
#ifdef WIN32
#pragma warning(pop)
#endif // WIN32

#ifdef EVTHREAD_USE_WINDOWS_THREADS_IMPLEMENTED
#define XPL_EVTHREAD_INITIALIZE() evthread_use_windows_threads()
#elif EVTHREAD_USE_PTHREADS_IMPLEMENTED
#define XPL_EVTHREAD_INITIALIZE() evthread_use_pthreads()
#else
#define XPL_EVTHREAD_INITIALIZE() \
do { \
} while (0)
#endif

namespace ngs {

class Connection_acceptor_socket : public xpl::iface::Connection_acceptor {
Expand Down Expand Up @@ -116,6 +130,11 @@ struct Socket_events::Socket_data {
};

Socket_events::Socket_events() {
static std::once_flag flag_event_threads_initialized;

std::call_once(flag_event_threads_initialized,
[]() { XPL_EVTHREAD_INITIALIZE(); });

m_evbase = event_base_new();

if (!m_evbase) throw std::bad_alloc();
Expand Down Expand Up @@ -173,7 +192,7 @@ void Socket_events::add_timer(const std::size_t delay_ms,

void Socket_events::loop() { event_base_loop(m_evbase, 0); }

void Socket_events::break_loop() { event_base_loopbreak(m_evbase); }
void Socket_events::break_loop() { event_base_loopexit(m_evbase, nullptr); }

void Socket_events::timeout_call(socket_type, short, void *arg) {
Timer_data *data = static_cast<Timer_data *>(arg);
Expand Down
3 changes: 3 additions & 0 deletions unittest/gunit/xplugin/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ SET(XPL_TEST_SRC
xpl/xmessage_buffer_t.cc
xpl/xrow_buffer_t.cc
xpl/xpl_regex_t.cc
xpl/socket_events_t.cc
)
# This compiler fails to handle some of the mock functions involved.
IF(MY_COMPILER_IS_SUNPRO)
Expand All @@ -148,6 +149,7 @@ MYSQL_ADD_EXECUTABLE(${XPL_UNIT_TESTS}
${XPL_TEST_SRC}
ADD_TEST xplugin
)
SET_TESTS_PROPERTIES(xplugin PROPERTIES TIMEOUT 180)

ADD_COMPILE_FLAGS(${XPL_TEST_SRC}
COMPILE_FLAGS "${MYSQLX_PROTOCOL_FLAGS}"
Expand Down Expand Up @@ -246,6 +248,7 @@ MYSQL_ADD_EXECUTABLE(${XCL_UNIT_TESTS}
${XCL_SRC}
ADD_TEST xclient
)
SET_TESTS_PROPERTIES(xclient PROPERTIES TIMEOUT 180)

ADD_DEPENDENCIES(${XCL_UNIT_TESTS}
${MYSQLX_CLIENT_FULL_LIB}
Expand Down
99 changes: 99 additions & 0 deletions unittest/gunit/xplugin/xpl/socket_events_t.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License, version 2.0,
as published by the Free Software Foundation.
This program is also distributed with certain software (including
but not limited to OpenSSL) that is licensed under separate terms,
as designated in a particular file or component or in included license
documentation. The authors of MySQL hereby grant you an additional
permission to link the program and your derivative works with the
separately licensed software that they have included with MySQL.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License, version 2.0, for more details.
You should have received a copy of the GNU General Public License
along with this program; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <thread>

#include "plugin/x/ngs/include/ngs/socket_events.h"

namespace ngs {
namespace test {

class Socket_events_task_suite : public ::testing::Test {
public:
Socket_events m_sut;
};

TEST_F(Socket_events_task_suite, loop_doesnt_block_when_no_events) {
m_sut.loop();
}

TEST_F(Socket_events_task_suite, execute_loop_until_no_events) {
int execution_count = 4;
m_sut.add_timer(10, [&execution_count]() { return --execution_count; });
m_sut.loop();
ASSERT_EQ(0, execution_count);
}

TEST_F(Socket_events_task_suite,
break_loop_is_queued_and_ignores_active_events) {
int execution_count = 0;

m_sut.break_loop();
m_sut.add_timer(10, [&execution_count]() {
++execution_count;
return true;
});
m_sut.loop();
ASSERT_EQ(0, execution_count);
}

TEST_F(Socket_events_task_suite, break_loop_from_thread) {
std::atomic<int> execution_count;

std::thread break_thread{[this, &execution_count]() {
while (execution_count.load() < 10) {
}
m_sut.break_loop();
}};

m_sut.add_timer(10, [&execution_count]() {
++execution_count;
return true;
});
m_sut.loop();
ASSERT_LT(0, execution_count.load());
break_thread.join();
}

TEST_F(Socket_events_task_suite, break_loop_from_thread_always_active) {
std::atomic<int> execution_count;

std::thread break_thread{[this, &execution_count]() {
while (execution_count.load() < 10) {
}
m_sut.break_loop();
}};

m_sut.add_timer(0, [&execution_count]() {
++execution_count;
std::this_thread::sleep_for(std::chrono::milliseconds(10));
return true;
});
m_sut.loop();
ASSERT_LT(0, execution_count.load());
break_thread.join();
}

} // namespace test
} // namespace ngs

0 comments on commit 2b84f0e

Please sign in to comment.