Fixed problems with server_context unit tests.

Added WITH_ASAN and WITH_TSAN options, enabled unit tests by default.

CMakeLists.txt

@@ -7,6 +7,12 @@ project (wsrep-lib)
 include(CheckIncludeFile)
 include(CTest)
 
+# Options
+option(WITH_AUTO_TEST "Run unit tests automatically after build" ON)
+option(WITH_ASAN "Enable address sanitizer" OFF)
+option(WITH_TSAN "Enable thread sanitizer" OFF)
+
+# CXX flags
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -Weffc++ -Woverloaded-virtual -Wno-non-virtual-dtor -g")
 
 check_include_file("${CMAKE_CURRENT_SOURCE_DIR}/wsrep/wsrep_api.h" HAVE_WSREP_API_HPP)
@@ -37,6 +43,13 @@ if (WITH_COVERAGE)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage")
   set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-arcs -ftest-coverage")
 endif()
+if (WITH_ASAN)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
+endif()
+if (WITH_TSAN)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=thread")
+endif()
+
 add_custom_target(coverage_report
   lcov --capture --directory . --output lcov.info --no-external
   COMMAND genhtml --output-directory coverage_report lcov.info)

include/wsrep/provider.hpp

@@ -16,6 +16,7 @@
 #include <string>
 #include <vector>
 #include <ostream>
+#include <sstream>
 
 namespace wsrep
 {
@@ -100,13 +101,6 @@ namespace wsrep
             assert(data_size <= 16);
             std::memcpy(data_, data, data_size);
         }
-#if 0
-        void assign(const void* data, size_t data_size)
-        {
-            assert(data_size == 16);
-            memcpy(data_, data, data_size);
-        }
-#endif
         bool operator<(const id& other) const
         {
             return (std::memcmp(data_, other.data_, sizeof(data_)) < 0);
@@ -375,6 +369,28 @@ namespace wsrep
         // Factory method
         static provider* make_provider(const std::string& provider);
     };
+
+    static inline std::string flags_to_string(int flags)
+    {
+        std::ostringstream oss;
+        if (flags & provider::flag::start_transaction)
+            oss << "start_transaction | ";
+        if (flags & provider::flag::commit)
+            oss << "commit | ";
+        if (flags & provider::flag::rollback)
+            oss << "rollback | ";
+        if (flags & provider::flag::isolation)
+            oss << "isolation | ";
+        if (flags & provider::flag::pa_unsafe)
+            oss << "pa_unsafe | ";
+        if (flags & provider::flag::snapshot)
+            oss << "snapshot | ";
+
+        std::string ret(oss.str());
+        if (ret.size() > 3) ret.erase(ret.size() - 3);
+        return ret;
+    }
+
 }
 
 #endif // WSREP_PROVIDER_HPP

include/wsrep/transaction_context.hpp

@@ -123,6 +123,7 @@ namespace wsrep
         wsrep::mutex& mutex();
 
         wsrep::ws_handle& ws_handle() { return ws_handle_; }
+        const wsrep::ws_meta& ws_meta() const { return ws_meta_; }
     private:
         transaction_context(const transaction_context&);
         transaction_context operator=(const transaction_context&);

src/CMakeLists.txt

@@ -27,6 +27,16 @@ add_test(NAME wsrep-lib_test
   COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/wsrep-lib_test
   )
 
+if (WITH_AUTO_TEST)
+  set(UNIT_TEST wsrep-lib_test)
+  add_custom_command(
+    TARGET ${UNIT_TEST}
+    COMMENT "Run tests"
+    POST_BUILD
+    COMMAND ${UNIT_TEST}
+    )
+endif()
+
 add_executable(dbms_simulator
   dbms_simulator.cpp)
 target_link_libraries(dbms_simulator wsrep-lib ${Boost_PROGRAM_OPTIONS_LIBRARY} ${Boost_SYSTEM_LIBRARY} ${Boost_FILESYSTEM_LIBRARY} ${Boost_THREAD_LIBRARY})

src/mock_utils.cpp

@@ -11,7 +11,7 @@
 void wsrep_mock::bf_abort_unordered(wsrep::client_context& cc)
 {
     wsrep::unique_lock<wsrep::mutex> lock(cc.mutex());
-    assert(cc.transaction().seqno() <= 0);
+    assert(cc.transaction().seqno().nil());
     cc.bf_abort(lock, 1);
 }
 
@@ -34,7 +34,8 @@ void wsrep_mock::start_applying_transaction(
     wsrep::ws_handle ws_handle(id, 0);
     wsrep::ws_meta ws_meta(wsrep::gtid(wsrep::id("1"), seqno),
                            wsrep::stid(wsrep::id("1"), id, cc.id()),
-                           flags, seqno.get() - 1);
+                           seqno.get() - 1, flags);
+    assert(ws_meta.flags());
     int ret(cc.start_transaction(ws_handle, ws_meta));
     if (ret != 0)
     {

src/server_context.cpp

@@ -18,20 +18,48 @@
 
 namespace
 {
-
+    inline uint32_t map_one(const int flags, const uint32_t from,
-    inline bool starts_transaction(uint32_t flags)
+                            const int to)
     {
-        return (flags & WSREP_FLAG_TRX_START);
+        return ((flags & from) ? to : 0);
     }
 
-    inline bool commits_transaction(uint32_t flags)
+    int map_flags_from_native(uint32_t flags)
     {
-        return (flags & WSREP_FLAG_TRX_END);
+        using wsrep::provider;
+        return (map_one(flags,
+                        WSREP_FLAG_TRX_START,
+                        provider::flag::start_transaction) |
+                map_one(flags,
+                        WSREP_FLAG_TRX_END,
+                        provider::flag::commit) |
+                map_one(flags,
+                        WSREP_FLAG_ROLLBACK,
+                        provider::flag::rollback) |
+                map_one(flags,
+                        WSREP_FLAG_ISOLATION,
+                        provider::flag::isolation) |
+                map_one(flags,
+                        WSREP_FLAG_PA_UNSAFE,
+                        provider::flag::pa_unsafe) |
+                // map_one(flags, provider::flag::commutative, WSREP_FLAG_COMMUTATIVE) |
+                // map_one(flags, provider::flag::native, WSREP_FLAG_NATIVE) |
+                map_one(flags, WSREP_FLAG_SNAPSHOT, provider::flag::snapshot));
     }
 
-    inline bool rolls_back_transaction(uint32_t flags)
+    inline bool starts_transaction(const wsrep::ws_meta& ws_meta)
     {
-        return (flags & WSREP_FLAG_ROLLBACK);
+        return (ws_meta.flags() & wsrep::provider::flag::start_transaction);
+    }
+
+    inline bool commits_transaction(const wsrep::ws_meta& ws_meta)
+    {
+        return (ws_meta.flags() & wsrep::provider::flag::commit);
+    }
+
+    inline bool rolls_back_transaction(const wsrep::ws_meta& ws_meta)
+    {
+        return (ws_meta.flags() & wsrep::provider::flag::rollback);
     }
 
     wsrep_cb_status_t connected_cb(
@@ -122,7 +150,8 @@ namespace
             wsrep::stid(wsrep::id(meta->stid.node.data,
                                   sizeof(meta->stid.node.data)),
                         meta->stid.trx,
-                        meta->stid.conn), meta->depends_on, flags);
+                        meta->stid.conn), meta->depends_on,
+            map_flags_from_native(flags));
         if (client_context->transaction().state() !=
             wsrep::transaction_context::s_replaying &&
             client_context->start_transaction(ws_handle, ws_meta))
@@ -312,8 +341,11 @@ int wsrep::server_context::on_apply(
 {
     int ret(0);
     const wsrep::transaction_context& txc(client_context.transaction());
-    if (starts_transaction(txc.flags()) &&
+    const wsrep::ws_meta& ws_meta(txc.ws_meta());
-        commits_transaction(txc.flags()))
+    // wsrep::log_debug() << "server_context::on apply flags: "
+    //                  << flags_to_string(ws_meta.flags());
+    assert(ws_meta.flags());
+    if (starts_transaction(ws_meta) && commits_transaction(ws_meta))
     {
         bool not_replaying(txc.state() !=
                            wsrep::transaction_context::s_replaying);
@@ -332,6 +364,12 @@ int wsrep::server_context::on_apply(
         {
             ret = 1;
         }
+
+        if (ret)
+        {
+            client_context.rollback();
+        }
+
         if (not_replaying)
         {
             client_context.after_statement();
@@ -347,12 +385,6 @@ int wsrep::server_context::on_apply(
         assert(0);
     }
 
-    if (ret)
-    {
-        client_context.rollback();
-    }
-
-    client_context.after_statement();
     assert(txc.active() == false);
     return ret;
 }

src/server_context_test.cpp

@@ -7,36 +7,46 @@
 
 #include <boost/test/unit_test.hpp>
 
-// Test on_apply() method for 1pc
+namespace
-BOOST_AUTO_TEST_CASE(server_context_applying_1pc)
 {
-    wsrep::mock_server_context sc("s1", "s1",
+    struct applying_server_fixture
-                                  wsrep::server_context::rm_sync);
+    {
-    wsrep::mock_client_context cc(sc,
+        applying_server_fixture()
+            : sc("s1", "s1",
+                 wsrep::server_context::rm_sync)
+            , cc(sc,
                  wsrep::client_id(1),
                  wsrep::client_context::m_applier,
-                                  false);
+                 false)
+        {
             wsrep_mock::start_applying_transaction(
                 cc, 1, 1,
-        wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit);
+                wsrep::provider::flag::start_transaction |
+                wsrep::provider::flag::commit);
+        }
+        wsrep::mock_server_context sc;
+        wsrep::mock_client_context cc;
+    };
+}
+
+// Test on_apply() method for 1pc
+BOOST_FIXTURE_TEST_CASE(server_context_applying_1pc,
+                        applying_server_fixture)
+{
+    cc.debug_log_level(1);
     char buf[1] = { 1 };
     BOOST_REQUIRE(sc.on_apply(cc, wsrep::data(buf, 1)) == 0);
     const wsrep::transaction_context& txc(cc.transaction());
-    BOOST_REQUIRE(txc.state() == wsrep::transaction_context::s_committed);
+    // ::abort();
+    BOOST_REQUIRE_MESSAGE(
+        txc.state() == wsrep::transaction_context::s_committed,
+        "Transaction state " << txc.state() << " not committed");
 }
 
 // Test on_apply() method for 2pc
-BOOST_AUTO_TEST_CASE(server_context_applying_2pc)
+BOOST_FIXTURE_TEST_CASE(server_context_applying_2pc,
+                        applying_server_fixture)
 {
-    wsrep::mock_server_context sc("s1", "s1",
-                                  wsrep::server_context::rm_sync);
-    wsrep::mock_client_context cc(sc,
-                                  wsrep::client_id(1),
-                                  wsrep::client_context::m_applier,
-                                  true);
-    wsrep_mock::start_applying_transaction(
-        cc, 1, 1,
-        wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit);
     char buf[1] = { 1 };
     BOOST_REQUIRE(sc.on_apply(cc, wsrep::data(buf, 1)) == 0);
     const wsrep::transaction_context& txc(cc.transaction());
@@ -45,20 +55,11 @@ BOOST_AUTO_TEST_CASE(server_context_applying_2pc)
 
 // Test on_apply() method for 1pc transaction which
 // fails applying and rolls back
-BOOST_AUTO_TEST_CASE(server_context_applying_1pc_rollback)
+BOOST_FIXTURE_TEST_CASE(server_context_applying_1pc_rollback,
+                        applying_server_fixture)
 {
-    wsrep::mock_server_context sc("s1", "s1",
-                                  wsrep::server_context::rm_sync);
-    wsrep::mock_client_context cc(sc,
-                                  wsrep::client_id(1),
-                                  wsrep::client_context::m_applier,
-                                  false);
     cc.fail_next_applying(true);
-    wsrep_mock::start_applying_transaction(
-        cc, 1, 1,
-        wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit);
     char buf[1] = { 1 };
-
     BOOST_REQUIRE(sc.on_apply(cc, wsrep::data(buf, 1)) == 1);
     const wsrep::transaction_context& txc(cc.transaction());
     BOOST_REQUIRE(txc.state() == wsrep::transaction_context::s_aborted);
@@ -66,18 +67,10 @@ BOOST_AUTO_TEST_CASE(server_context_applying_1pc_rollback)
 
 // Test on_apply() method for 2pc transaction which
 // fails applying and rolls back
-BOOST_AUTO_TEST_CASE(server_context_applying_2pc_rollback)
+BOOST_FIXTURE_TEST_CASE(server_context_applying_2pc_rollback,
+                        applying_server_fixture)
 {
-    wsrep::mock_server_context sc("s1", "s1",
-                                  wsrep::server_context::rm_sync);
-    wsrep::mock_client_context cc(sc,
-                                  wsrep::client_id(1),
-                                  wsrep::client_context::m_applier,
-                                  true);
     cc.fail_next_applying(true);
-    wsrep_mock::start_applying_transaction(
-        cc, 1, 1,
-        wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit);
     char buf[1] = { 1 };
     BOOST_REQUIRE(sc.on_apply(cc, wsrep::data(buf, 1)) == 1);
     const wsrep::transaction_context& txc(cc.transaction());

src/transaction_context.cpp

@@ -64,6 +64,7 @@ int wsrep::transaction_context::start_transaction(
     const wsrep::ws_handle& ws_handle,
     const wsrep::ws_meta& ws_meta)
 {
+    assert(ws_meta.flags());
     assert(active() == false);
     assert(client_context_.mode() == wsrep::client_context::m_applier);
     state_ = s_executing;
@@ -557,6 +558,12 @@ void wsrep::transaction_context::state(
     wsrep::unique_lock<wsrep::mutex>& lock __attribute__((unused)),
     enum wsrep::transaction_context::state next_state)
 {
+    if (client_context_.debug_log_level() >= 1)
+    {
+        log_debug() << "client: " << client_context_.id().get()
+                    << " txc: " << id().get()
+                    << " state: " << state_ << " -> " << next_state;
+    }
     assert(lock.owns_lock());
     static const char allowed[n_states][n_states] =
         { /*  ex pr ce co oc ct cf ma ab ad mr re */

src/transaction_context_test.cpp

@@ -376,28 +376,9 @@ BOOST_FIXTURE_TEST_CASE(transaction_context_1pc_applying,
     BOOST_REQUIRE(cc.current_error() == wsrep::e_success);
 }
 
-BOOST_AUTO_TEST_CASE(transaction_context_2pc_applying)
+BOOST_FIXTURE_TEST_CASE(transaction_context_2pc_applying,
+                        applying_client_fixture)
 {
-    wsrep::mock_server_context sc("s1", "s1",
-                                  wsrep::server_context::rm_sync);
-    wsrep::mock_client_context cc(sc,
-                             wsrep::client_id(1),
-                             wsrep::client_context::m_applier);
-
-    BOOST_REQUIRE(cc.before_command() == 0);
-    BOOST_REQUIRE(cc.before_statement() == 0);
-
-    wsrep_mock::start_applying_transaction(
-        cc, 1, 1,
-        wsrep::provider::flag::start_transaction | wsrep::provider::flag::commit);
-    const wsrep::transaction_context& tc(cc.transaction());
-
-    BOOST_REQUIRE(tc.active() == false);
-    BOOST_REQUIRE(cc.start_transaction() == 0);
-    BOOST_REQUIRE(tc.active() == true);
-    BOOST_REQUIRE(tc.certified() == true);
-    BOOST_REQUIRE(tc.ordered() == true);
-
     BOOST_REQUIRE(cc.before_prepare() == 0);
     BOOST_REQUIRE(tc.state() == wsrep::transaction_context::s_preparing);
     BOOST_REQUIRE(cc.after_prepare() == 0);

src/wsrep_provider_v26.cpp

@@ -119,8 +119,8 @@ namespace
 
         ~const_ws_handle()
         {
-            assert(ws_handle.transaction_id().get() == native_.trx_id);
+            assert(ws_handle_.transaction_id().get() == native_.trx_id);
-            assert(ws_handle.opaque() == native_.opaque);
+            assert(ws_handle_.opaque() == native_.opaque);
         }
 
         const wsrep_ws_handle_t* native() const