Fix/tsan (#3617)

* fix(Foundation): tsan warnings fixes * fix(Thread_POSIX): tsan warnings fixes; add tsan.suppress * fix(Util): tsan fixes * fix(netSSL_OpenSSL): tsan fixes * fix(Data): tsan warnings fixes * feat(ci): add tsan job * feat(ci): add tsan job, another attempt * feat(ci): add tsan job, 3rd attempt * fix(Foundation): tsan warnings fixes * fix(Thread_POSIX): tsan warnings fixes; add tsan.suppress * fix(Util): tsan fixes * fix(netSSL_OpenSSL): tsan fixes * fix(Data): tsan warnings fixes * feat(ci): add tsan job * feat(ci): add tsan job, another attempt * feat(ci): add tsan job, 3rd attempt * fix(ResultMetadata): memory leak #3474 * feat(ci): disable ActiveDispatcher tests for tsan runs * feat(ci): try to fix tsan options file detection (again) * chore(TestLibrary: correct spelling * fix(ci): fix tsan run; add -y to apt; disable samples build for some jobs * fix(ci): add mysql ports * feat(ci): add VS asan * feat(double-conversion): Upgrade double-conversion to v3.2.0 #3624 * chore(asan): disable msvc asan build (dll not found) * chore(double-conversion): move NumericString.h before double-conversion includes to prevent min/max collision; reinstate lost loongarch64 * chore(JSON): sync pdjson with upstream * fix(Statement): Poco::Data::Statement becomes unusable after exception #2287
pocoproject · Jun 2, 2022 · ff879f5 · ff879f5
1 parent 17fec1b
commit ff879f5
Show file tree

Hide file tree

Showing 55 changed files with 2,331 additions and 1,759 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,7 +15,7 @@ jobs:
           - 3306:3306
     steps:
       - uses: actions/checkout@v2
-      - run: sudo apt update && sudo apt install libssl-dev unixodbc-dev redis-server libmysqlclient-dev
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev redis-server libmysqlclient-dev
       - run: ./configure --everything --omit=PDF && make all -s -j4 && sudo make install
       - run: >-
           sudo -s
@@ -24,21 +24,31 @@ jobs:
 
   linux-gcc-make-asan:
     runs-on: ubuntu-20.04
+    services:
+      mysql:
+        image: mysql:latest
+        env:
+          MYSQL_ALLOW_EMPTY_PASSWORD: yes
+          MYSQL_USER: pocotest
+          MYSQL_PASSWORD: pocotest
+          MYSQL_DATABASE: pocotest
+        ports:
+          - 3306:3306
     steps:
       - uses: actions/checkout@v2
-      - run: sudo apt update && sudo apt install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
-      - run: ./configure --everything --omit=PDF && make all -s -j4 SANITIZEFLAGS=-fsanitize=address && sudo make install
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
+      - run: ./configure --everything --no-samples --omit=PDF && make all -s -j4 SANITIZEFLAGS=-fsanitize=address && sudo make install
       - run: >-
           sudo -s
-          EXCLUDE_TESTS="Data/MySQL Data/ODBC Data/PostgreSQL MongoDB"
+          EXCLUDE_TESTS="Data/ODBC Data/PostgreSQL MongoDB"
           ./ci/runtests.sh
 
   linux-gcc-make-asan-no-soo:
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v2
-      - run: sudo apt update && sudo apt install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
-      - run: ./configure --everything --omit=PDF --no-soo && make all -s -j4 SANITIZEFLAGS=-fsanitize=address && sudo make install
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
+      - run: ./configure --everything --no-samples --omit=PDF --no-soo && make all -s -j4 SANITIZEFLAGS=-fsanitize=address && sudo make install
       - run: >-
           sudo -s
           EXCLUDE_TESTS="Data/MySQL Data/ODBC Data/PostgreSQL MongoDB"
@@ -48,18 +58,28 @@ jobs:
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v2
-      - run: sudo apt update && sudo apt install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
-      - run: ./configure --everything --omit=PDF && make all -s -j4 SANITIZEFLAGS=-fsanitize=undefined && sudo make install
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
+      - run: ./configure --everything --no-samples --omit=PDF && make all -s -j4 SANITIZEFLAGS=-fsanitize=undefined && sudo make install
       - run: >-
           sudo -s
           EXCLUDE_TESTS="Data/MySQL Data/ODBC Data/PostgreSQL MongoDB"
           ./ci/runtests.sh
 
+  linux-gcc-make-tsan:
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev redis-server
+      - run: ./configure --everything --no-samples --omit=CppParser,Encodings,Data/MySQL,Data/ODBC,Data/PostgreSQL,MongoDB,PageCompiler,PDF,PocoDoc,ProGen,Redis,SevenZip && make all -s -j4 SANITIZEFLAGS=-fsanitize=thread && sudo make install
+      - run: >-
+          sudo -s
+          ./ci/runtests.sh TSAN
+
   linux-gcc-cmake:
     runs-on: ubuntu-20.04
     steps:
       - uses: actions/checkout@v2
-      - run: sudo apt update && sudo apt install cmake ninja-build libssl-dev unixodbc-dev libmysqlclient-dev redis-server
+      - run: sudo apt -y update && sudo apt -y install cmake ninja-build libssl-dev unixodbc-dev libmysqlclient-dev redis-server
       - run: cmake -H. -Bcmake-build -GNinja -DENABLE_PDF=OFF -DENABLE_TESTS=ON && cmake --build cmake-build --target all
       - run: >-
           cd cmake-build &&
@@ -72,8 +92,8 @@ jobs:
     steps:
       - uses: actions/checkout@v2
       - run: >-
-          sudo apt-get update &&
-          sudo apt-get install crossbuild-essential-armhf
+          sudo apt-get -y update &&
+          sudo apt-get -y install crossbuild-essential-armhf
       - run: >-
           ./configure --config=ARM-Linux --everything --omit=PDF,Crypto,NetSSL_OpenSSL,JWT,Data/MySQL,Data/ODBC,Data/PostgreSQL,PageCompiler,PageCompiler/File2Page &&
           make all -s -j4 ARCHFLAGS="-mcpu=cortex-a8 -mfloat-abi=hard -mfpu=neon" TOOL=arm-linux-gnueabihf
@@ -139,3 +159,16 @@ jobs:
       - run: >-
           cd cmake-build;
           ctest --output-on-failure -E "(DataMySQL)|(DataODBC)|(Redis)|(MongoDB)" -C Release
+
+# missing asan dll path
+#  windows-2022-msvc-cmake-2022-asan:
+#    runs-on: windows-2022
+#    env:
+#      CPPUNIT_IGNORE: class CppUnit::TestCaller<class PathTest>.testFind,class CppUnit::TestCaller<class ICMPSocketTest>.testSendToReceiveFrom,class CppUnit::TestCaller<class ICMPClientTest>.testPing,class CppUnit::TestCaller<class ICMPClientTest>.testBigPing,class CppUnit::TestCaller<class ICMPSocketTest>.testMTU,class CppUnit::TestCaller<class HTTPSClientSessionTest>.testProxy,class CppUnit::TestCaller<class HTTPSStreamFactoryTest>.testProxy
+#    steps:
+#      - uses: actions/checkout@v2
+#      - run: cmake -S. -Bcmake-build -DPOCO_SANITIZE_ASAN=ON -DENABLE_NETSSL_WIN=ON -DENABLE_NETSSL=OFF -DENABLE_CRYPTO=OFF -DENABLE_JWT=OFF -DENABLE_DATA=ON -DENABLE_DATA_ODBC=ON -DENABLE_DATA_MYSQL=OFF -DENABLE_DATA_POSTGRESQL=OFF -DENABLE_TESTS=ON
+#      - run: cmake --build cmake-build --config Debug
+#      - run: >-
+#          cd cmake-build;
+#          ctest --output-on-failure -E "(DataMySQL)|(DataODBC)|(Redis)|(MongoDB)" -C Debug
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -78,6 +78,10 @@ if(MSVC)
 	if((NOT POCO_DISABLE_INTERNAL_OPENSSL) AND (ENABLE_NETSSL OR ENABLE_CRYPTO OR (ENABLE_DATA_MYSQL AND MINGW)))
 		include(UseEmbeddedOpenSSL)
 	endif()
+
+	if(POCO_SANITIZE_ASAN)
+		SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /fsanitize=address")
+	endif()
 endif()
 
 option(ENABLE_NETSSL_WIN "Enable NetSSL Windows" OFF)

diff --git a/Data/MySQL/include/Poco/Data/MySQL/ResultMetadata.h b/Data/MySQL/include/Poco/Data/MySQL/ResultMetadata.h
@@ -71,6 +71,8 @@ class ResultMetadata
 		/// Expands the size allocated for column to fit the length of the data.
 
 private:
+	void freeMemory();
+
 	std::vector<MetaColumn>    _columns;
 	std::vector<MYSQL_BIND>    _row;
 	std::vector<char*>         _buffer;

diff --git a/Data/MySQL/src/ResultMetadata.cpp b/Data/MySQL/src/ResultMetadata.cpp
@@ -147,13 +147,13 @@ namespace MySQL {
 
 ResultMetadata::~ResultMetadata()
 {
-	for (std::vector<char*>::iterator it = _buffer.begin(); it != _buffer.end(); ++it)
-		std::free(*it);
+	freeMemory();
 }
 
 
 void ResultMetadata::reset()
 {
+	freeMemory();
 	_columns.resize(0);
 	_row.resize(0);
 	_buffer.resize(0);
@@ -162,14 +162,20 @@ void ResultMetadata::reset()
 }
 
 
+void ResultMetadata::freeMemory()
+{
+	for (std::vector<char*>::iterator it = _buffer.begin(); it != _buffer.end(); ++it)
+		std::free(*it);
+}
+
+
 void ResultMetadata::init(MYSQL_STMT* stmt)
 {
 	ResultMetadataHandle h(stmt);
 
 	if (!h)
 	{
-		// all right, it is normal
-		// querys such an "INSERT INTO" just does not have result at all
+		// some queries (eg. INSERT) don't have result
 		reset();
 		return;
 	}

diff --git a/Data/include/Poco/Data/Statement.h b/Data/include/Poco/Data/Statement.h
@@ -78,6 +78,7 @@ class Data_API Statement
 	using ResultPtr = SharedPtr<Result>;
 	using AsyncExecMethod = ActiveMethod<std::size_t, bool, StatementImpl>;
 	using AsyncExecMethodPtr = SharedPtr<AsyncExecMethod>;
+	using State = StatementImpl::State;
 
 	static const int WAIT_FOREVER = -1;
 
@@ -385,6 +386,9 @@ class Data_API Statement
 		/// Sets the row formatter for this statement.
 		/// Statement takes the ownership of the formatter.
 
+	State state() const;
+		/// Returns the statement state.
+
 protected:
 	using ImplPtr = StatementImpl::Ptr;
 
@@ -791,6 +795,12 @@ inline bool Statement::isAsync() const
 }
 
 
+inline Statement::State Statement::state() const
+{
+	return _pImpl->getState();
+}
+
+
 inline void Statement::setRowFormatter(RowFormatter::Ptr pRowFormatter)
 {
 	_pRowFormatter = pRowFormatter;

diff --git a/Data/include/Poco/Data/StatementImpl.h b/Data/include/Poco/Data/StatementImpl.h
@@ -432,7 +432,7 @@ class Data_API StatementImpl
 
 	using CountVec = std::vector<std::size_t>;
 
-	State                    _state;
+	std::atomic<State>       _state;
 	Limit                    _extrLimit;
 	std::size_t              _lowerLimit;
 	std::vector<int>         _columnsExtracted;

diff --git a/Data/src/StatementImpl.cpp b/Data/src/StatementImpl.cpp
@@ -69,34 +69,40 @@ StatementImpl::~StatementImpl()
 
 std::size_t StatementImpl::execute(const bool& reset)
 {
-	if (reset) resetExtraction();
+	std::size_t lim = 0;
 
-	if (!_rSession.isConnected())
+	try
 	{
-		_state = ST_DONE;
-		throw NotConnectedException(_rSession.connectionString());
-	}
+		if (reset) resetExtraction();
 
-	std::size_t lim = 0;
-	if (_lowerLimit > _extrLimit.value())
-		throw LimitException("Illegal Statement state. Upper limit must not be smaller than the lower limit.");
+		if (!_rSession.isConnected())
+			throw NotConnectedException(_rSession.connectionString());
 
-	do
-	{
-		compile();
-		if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
-			lim += executeWithoutLimit();
-		else
-			lim += executeWithLimit();
-	} while (canCompile());
+		if (_lowerLimit > _extrLimit.value())
+			throw LimitException("Illegal Statement state. Upper limit must not be smaller than the lower limit.");
 
-	if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
-		_state = ST_DONE;
+		do
+		{
+			compile();
+			if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
+				lim += executeWithoutLimit();
+			else
+				lim += executeWithLimit();
+		} while (canCompile());
+
+		if (_extrLimit.value() == Limit::LIMIT_UNLIMITED)
+			_state = ST_DONE;
 
-	if (lim < _lowerLimit)
-		throw LimitException("Did not receive enough data.");
+		if (lim < _lowerLimit)
+			throw LimitException("Did not receive enough data.");
 
-	assignSubTotal(reset);
+		assignSubTotal(reset);
+	}
+	catch(...)
+	{
+		_state = ST_DONE;
+		throw;
+	}
 
 	return lim;
 }

diff --git a/Data/testsuite/src/DataTest.cpp b/Data/testsuite/src/DataTest.cpp
@@ -127,6 +127,8 @@ void DataTest::testSession()
 		fail ("must fail");
 	} catch (NotConnectedException&) { }
 
+	assertTrue(stmt.done());
+
 	try
 	{
 		sess << "SELECT * FROM Strings", now; 
@@ -137,6 +139,21 @@ void DataTest::testSession()
 	assertTrue (sess.getFeature("connected"));
 	assertTrue (sess.isConnected());
 
+	// ensure that throwing during execution leaves
+	// statement in valid state (ST_DONE)
+	sess.setFeature("throwOnHasNext", true);
+	Statement stmt1 = (sess << "SELECT * FROM Strings", into(str), limit(50));
+	assertTrue (sess.getFeature("throwOnHasNext"));
+	try
+	{
+		stmt1.execute();
+		fail ("must trow UnknownDataBaseException");
+	}
+	catch(const Poco::Data::UnknownDataBaseException&) {}
+	assertTrue(stmt1.done());
+
+	// reset session back to normal operation
+	sess.setFeature("throwOnHasNext", false);
 	sess << "SELECT * FROM Strings", now; 
 	stmt.execute();
 

diff --git a/Data/testsuite/src/SessionImpl.cpp b/Data/testsuite/src/SessionImpl.cpp
@@ -26,6 +26,7 @@ SessionImpl::SessionImpl(const std::string& init, std::size_t timeout):
 	addFeature("f1", &SessionImpl::setF, &SessionImpl::getF);
 	addFeature("f2", 0, &SessionImpl::getF);
 	addFeature("f3", &SessionImpl::setF, 0);
+	addFeature("throwOnHasNext", &SessionImpl::setThrowOnHasNext, &SessionImpl::getThrowOnHasNext);
 	addFeature("connected", &SessionImpl::setConnected, &SessionImpl::getConnected);
 	addProperty("p1", &SessionImpl::setP, &SessionImpl::getP);
 	addProperty("p2", 0, &SessionImpl::getP);
@@ -73,7 +74,7 @@ std::size_t SessionImpl::getConnectionTimeout() const
 
 StatementImpl::Ptr SessionImpl::createStatementImpl()
 {
-	return new TestStatementImpl(*this);
+	return new TestStatementImpl(*this, _throwOnHasNext);
 }
 
 
@@ -139,13 +140,13 @@ bool SessionImpl::getConnected(const std::string& name) const
 }
 
 
-void SessionImpl::setConnected(const std::string& name, bool value)
+void SessionImpl::setConnected(const std::string&, bool value)
 {
 	_connected = value;
 }
 
 
-void SessionImpl::setF(const std::string& name, bool value)
+void SessionImpl::setF(const std::string&, bool value)
 {
 	_f = value;
 }
@@ -157,6 +158,17 @@ bool SessionImpl::getF(const std::string& name) const
 }
 
 
+void SessionImpl::setThrowOnHasNext(const std::string&, bool value)
+{
+	_throwOnHasNext = value;
+}
+
+
+bool SessionImpl::getThrowOnHasNext(const std::string& name) const
+{
+	return _throwOnHasNext;
+}
+
 void SessionImpl::setP(const std::string& name, const Poco::Any& value)
 {
 	_p = value;

diff --git a/Data/testsuite/src/SessionImpl.h b/Data/testsuite/src/SessionImpl.h
@@ -98,11 +98,14 @@ class SessionImpl: public Poco::Data::AbstractSessionImpl<SessionImpl>
 
 	void setF(const std::string& name, bool value);
 	bool getF(const std::string& name) const;
+	void setThrowOnHasNext(const std::string& name, bool value);
+	bool getThrowOnHasNext(const std::string& name) const;
 	void setP(const std::string& name, const Poco::Any& value);
 	Poco::Any getP(const std::string& name) const;
 
 private:
 	bool         _f;
+	bool         _throwOnHasNext = false;
 	Poco::Any    _p;
 	bool         _connected;
 	std::string  _connectionString;

diff --git a/Data/testsuite/src/TestStatementImpl.cpp b/Data/testsuite/src/TestStatementImpl.cpp
@@ -17,9 +17,10 @@ namespace Data {
 namespace Test {
 
 
-TestStatementImpl::TestStatementImpl(SessionImpl& rSession):
+TestStatementImpl::TestStatementImpl(SessionImpl& rSession, bool throwOnHasNext):
 	Poco::Data::StatementImpl(rSession),
-	_compiled(false)
+	_compiled(false),
+	_throwOnHasNext(throwOnHasNext)
 {
 }
 
@@ -79,6 +80,8 @@ const MetaColumn& TestStatementImpl::metaColumn(std::size_t pos) const
 
 bool TestStatementImpl::hasNext()
 {
+	if (_throwOnHasNext)
+		throw Poco::Data::UnknownDataBaseException();
 	return false;
 }