From: Vamsikrishna Bhagi Date: October 18 2012 5:36am Subject: bzr push into mysql-5.6 branch (vamsikrishna.bhagi:4500 to 4501) Bug#14547920 List-Archive: http://lists.mysql.com/commits/145076 X-Bug: 14547920 Message-Id: <20121018053622.27664.58203.4501@vamsibhagi> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4501 Vamsikrishna Bhagi 2012-10-18 Bug#14547920 I_MAIN.BUG13115401 CAUSING VALGRIND REPORT FAILURE ON PB2 Issue: The test i_main.bug13115401 which was added to fix the bug #13115401 has introduced a sproadic valgrind failure report on pushbuild specific to yaSSL build. It being a sporadic issue, it can be reproduced once or twice when the command below is run for around 50 times. perl mysql-test-run.pl --timer --debug-server --force --comment=n_mix-debug --vardir=var-n_mix --mysqld=--binlog-format=mixed --experimental=collections/default.experimental --skip-ndb --skip-test-list=collections/disabled-per-push.list --unit-tests --valgrind --valgrind-option=--gen-suppressions=all --valgrind-option=--show-reachable=yes Solution: The problem arises on a multi-threaded scenario when a thread tries to create a session object from GetSessions() method as another thread is already doing a creation. As the memory allocated in first thread is overridden by memory allocated by the second thread, the memory allocated by the first thread will be a leak in this case. This is because the yassl session instance is not thread safe. In the current scenario a pointer is not necessary as it is created from a singleton method and actually adds to complexity by allowing multiple allocations. Hence it is replaced by a normal object. This will solve the problem aroused in a multithreaded scenario. ****** Bug#14547920 I_MAIN.BUG13115401 CAUSING VALGRIND REPORT FAILURE ON PB2 Issue: The test i_main.bug13115401 which was added to fix the bug #13115401 has introduced a sporadic valgrind failure report on pushbuild specific to yaSSL build. Being a sporadic issue, it can be reproduced once or twice when the command below is run for around 50 times. perl mysql-test-run.pl --timer --debug-server --force --comment=n_mix-debug --vardir=var-n_mix --mysqld=--binlog-format=mixed --experimental=collections/default.experimental --skip-ndb --skip-test-list=collections/disabled-per-push.list --unit-tests --valgrind --valgrind-option=--gen-suppressions=all --valgrind-option=--show-reachable=yes Solution: The problem arises on a multi-threaded scenario when a thread tries to create a session object from GetSessions() method as another thread is already doing a creation. As the memory allocated in first thread is overridden by memory allocated by the second thread, the memory allocated by the first thread will be a leak in this case. This is because the yassl session instance is not thread safe. To make this instance creation process compatible with multi-threading, a new instance is created by with the help of pthread_once, which will essentially check if the function used for creating an instance is already called previously or not. @ extra/yassl/src/yassl_int.cpp Bug#14547920 I_MAIN.BUG13115401 CAUSING VALGRIND REPORT FAILURE ON PB2 The return value of the method GetSession() is a pointer *sessionsInstance which is created afterchecking if the pointer is empty or not. But this results in a leak when a thread execution clears the check condition if (!sessionsInstance) as the allocation for another thread is going on at sessionsInstance = NEW_YS Sessions. Implementing this method with a pointer causes this conflict. A pointer is not required in this case as the allocation is done through a singleton creator and is also adding a loophole in a multithreaded condition, hence it is replaced with normal object Sessions::sessionsInstance. ****** Bug#14547920 I_MAIN.BUG13115401 CAUSING VALGRIND REPORT FAILURE ON PB2 The return value of the method GetSession() is a pointer *sessionsInstance which is created after checking if the pointer is empty or not. But this results in a leak when a thread execution clears the check condition if (!sessionsInstance) as the allocation for another thread is going on at sessionsInstance = NEW_YS Sessions. To make this process thread safe, pthread_once method is used to create an instance which will see to that session is created only once in existing context(kindly refer C++ documentation for more details on pthread_once). Since pthread_once takes only void returning functions as an argument, a new function Session_initialize is implemented to create a session. The check variable session_created is refreshed after deleting the sessionInstance in yaSSL cleanup. Since a session is being created only after the old one is cleared, there will not be a memory leak. modified: extra/yassl/include/yassl_int.hpp extra/yassl/src/yassl_int.cpp 4500 Amit Bhattacharya 2012-10-18 Removing the files to fix the issues removed: mysql-test/include/multi_table_del_sj.inc mysql-test/include/multi_table_upd_sj.inc mysql-test/r/multi_table_upd_del_sj.result mysql-test/t/multi_table_upd_del_sj.test === modified file 'extra/yassl/include/yassl_int.hpp' --- a/extra/yassl/include/yassl_int.hpp 2012-07-24 13:24:00 +0000 +++ b/extra/yassl/include/yassl_int.hpp 2012-10-18 05:32:59 +0000 @@ -278,13 +278,13 @@ public: ~Sessions(); + friend void Session_initialize(); friend Sessions& GetSessions(); // singleton creator private: Sessions(const Sessions&); // hide copy Sessions& operator=(const Sessions&); // and assign }; - #ifdef _POSIX_THREADS typedef pthread_t THREAD_ID_T; #else === modified file 'extra/yassl/src/yassl_int.cpp' --- a/extra/yassl/src/yassl_int.cpp 2012-07-24 13:24:00 +0000 +++ b/extra/yassl/src/yassl_int.cpp 2012-10-18 05:32:59 +0000 @@ -17,15 +17,12 @@ * draft along with type conversion functions. */ +#include "pthread.h" #include "runtime.hpp" #include "yassl_int.hpp" #include "handshake.hpp" #include "timer.hpp" -#ifdef _POSIX_THREADS - #include "pthread.h" -#endif - #ifdef HAVE_LIBZ #include "zlib.h" @@ -1564,17 +1561,22 @@ SSL_SESSION::~SSL_SESSION() ysDelete(peerX509_); } - static Sessions* sessionsInstance = 0; +static pthread_once_t session_created= PTHREAD_ONCE_INIT; + + +void Session_initialize() +{ + sessionsInstance = NEW_YS Sessions; +} + Sessions& GetSessions() { - if (!sessionsInstance) - sessionsInstance = NEW_YS Sessions; + pthread_once(&session_created, Session_initialize); return *sessionsInstance; } - static sslFactory* sslFactoryInstance = 0; sslFactory& GetSSL_Factory() @@ -2627,9 +2629,10 @@ extern "C" void yaSSL_CleanUp() { TaoCrypt::CleanUp(); yaSSL::ysDelete(yaSSL::sslFactoryInstance); - yaSSL::ysDelete(yaSSL::sessionsInstance); yaSSL::ysDelete(yaSSL::errorsInstance); - + yaSSL::ysDelete(yaSSL::sessionsInstance); + yaSSL::session_created= PTHREAD_ONCE_INIT; + // In case user calls more than once, prevent seg fault yaSSL::sslFactoryInstance = 0; yaSSL::sessionsInstance = 0; No bundle (reason: useless for push emails).