Merge pull request 'some fixes' (#198) from some_fixes into master

Reviewed-on: #198

.gitignore

@@ -1,5 +1,5 @@
 /src_main/piversion.h
-/.svn
+/.*
 /doc/rtf
 _unsused
 CMakeLists.txt.user*

libs/client_server/piclientserver_server.cpp

@@ -72,11 +72,7 @@ PIClientServer::Server::~Server() {
 	clean_thread->waitForFinish();
 	piDeleteSafety(clean_thread);
 	stopServer();
-	for (auto c: clients) {
-		c->aboutDelete();
-		c->destroy();
-		delete c;
-	}
+	closeAll();
 	piDeleteSafety(tcp_server);
 }
 

libs/main/io_devices/piethernet.cpp

@@ -527,6 +527,7 @@ bool PIEthernet::listen(bool threaded) {
 		listen_threaded = true;
 		server_bounded  = false;
 		server_thread_.start(server_func);
+		server_thread_.waitForStart();
 		return true;
 	}
 	listen_threaded = server_bounded = false;

libs/main/thread/piprotectedvariable.h

@@ -4,22 +4,22 @@
 //! \~english Thread-safe variable
 //! \~russian Потокобезопасная переменная
 /*
-	PIP - Platform Independent Primitives
-	Thread-safe variable
-	Ivan Pelipenko peri4ko@yandex.ru
+    PIP - Platform Independent Primitives
+    Thread-safe variable
+    Ivan Pelipenko peri4ko@yandex.ru
 
-	This program is free software: you can redistribute it and/or modify
-	it under the terms of the GNU Lesser General Public License as published by
-	the Free Software Foundation, either version 3 of the License, or
-	(at your option) any later version.
+    This program is free software: you can redistribute it and/or modify
+    it under the terms of the GNU Lesser General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
 
-	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 Lesser General Public License for more details.
+    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 Lesser General Public License for more details.
 
-	You should have received a copy of the GNU Lesser General Public License
-	along with this program.  If not, see <http://www.gnu.org/licenses/>.
+    You should have received a copy of the GNU Lesser General Public License
+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
 #ifndef PIPROTECTEDVARIABLE_H
@@ -34,21 +34,29 @@
 template<typename T>
 class PIP_EXPORT PIProtectedVariable {
 public:
+	//! \~english Constructs %PIProtectedVariable and initialize variable by value `v`.
+	//! \~russian Создает %PIProtectedVariable и инициализирует переменную значением `v`.
+	PIProtectedVariable(T v = T()): var(std::move(v)) {}
+
 	//! \~\brief
 	//! \~english Pointer-like wrapper returned by \a getRef() while the protected value remains locked.
 	//! \~russian Указателеподобная обертка, возвращаемая \a getRef(), пока защищенное значение остается заблокированным.
 	class PIP_EXPORT Pointer {
 		friend class PIProtectedVariable<T>;
+		NO_COPY_CLASS(Pointer);
+		Pointer & operator=(Pointer && other) = delete;
 
 	public:
-		//! \~english Copies wrapper state for access to the same protected value.
-		//! \~russian Копирует состояние обертки для доступа к тому же защищенному значению.
-		Pointer(const Pointer & v): pv(v.pv), counter(v.counter + 1) {}
+		//! \~english Move constructor - transfers ownership of the lock.
+		//! \~russian Конструктор перемещения - передает владение блокировкой.
+		Pointer(Pointer && other): pv(other.pv) { other.can_unlock = false; };
 
-		//! \~english Destroys wrapper and releases the mutex when it owns the original lock.
-		//! \~russian Уничтожает обертку и освобождает мьютекс, когда она владеет исходной блокировкой.
+		//! \~english Destroys wrapper and releases the mutex.
+		//! \~russian Уничтожает обертку и освобождает мьютекс.
 		~Pointer() {
-			if (counter == 0) pv.mutex.unlock();
+			if (can_unlock) {
+				pv.mutex.unlock();
+			}
 		}
 
 		//! \~english Returns pointer access to the protected value.
@@ -60,11 +68,11 @@ public:
 		T & operator*() { return pv.var; }
 
 	private:
-		Pointer() = delete;
-		Pointer(PIProtectedVariable<T> & v): pv(v) {}
+		explicit Pointer() = delete;
+		explicit Pointer(PIProtectedVariable<T> & v): pv(v) { pv.mutex.lock(); }
 
 		PIProtectedVariable<T> & pv;
-		int counter = 0;
+		bool can_unlock = true;
 	};
 
 	//! \~english Replaces the protected value with \a v.
@@ -76,10 +84,7 @@ public:
 
 	//! \~english Locks the value and returns wrapper-based access to it.
 	//! \~russian Блокирует значение и возвращает обертку для доступа к нему.
-	Pointer getRef() {
-		mutex.lock();
-		return Pointer(*this);
-	}
+	Pointer getRef() { return Pointer(*this); }
 
 	//! \~english Returns a copy of the protected value.
 	//! \~russian Возвращает копию защищенного значения.
@@ -98,7 +103,7 @@ public:
 
 private:
 	mutable PIMutex mutex;
-	T var;
+	T var = {};
 };
 
 

libs/main/types/pisystemtime.cpp

@@ -105,7 +105,7 @@ extern clock_serv_t __pi_mac_clock;
 //! Используйте этот метод для ожидания разниц системных времен или своего времени.
 //! Если метод будет вызван для системного времени \a PISystemTime::current(), то
 //! ожидание будет почти бесконечным
-void PISystemTime::sleep() {
+void PISystemTime::sleep() const {
 	piUSleep(piFloord(toMicroseconds()));
 }
 
@@ -118,7 +118,7 @@ void PISystemTime::toTimespec(void * ts) {
 }
 
 
-PISystemTime::Frequency PISystemTime::toFrequency() {
+PISystemTime::Frequency PISystemTime::toFrequency() const {
 	return PISystemTime::Frequency::fromSystemTime(*this);
 }
 

libs/main/types/pisystemtime.h

@@ -253,7 +253,7 @@ public:
 
 	//! \~english Sleep for this time
 	//! \~russian Ожидать это время
-	void sleep();
+	void sleep() const;
 
 	//! \~english On *nix system assign current value to timespec struct
 	//! \~russian На *nix системах присваивает время к timespec структуре
@@ -261,7 +261,7 @@ public:
 
 	//! \~english Returns \a Frequency that corresponds this time interval
 	//! \~russian Возвращает \a Frequency соответствующую этому временному интервалу
-	PISystemTime::Frequency toFrequency();
+	PISystemTime::Frequency toFrequency() const;
 
 	//! \~english Returns "yyyy-MM-dd hh:mm:ss.zzz" for absolute time and "<V> <d|h|m|s|ms|us|ns> ..." for relative
 	//! \~russian Возвращает "yyyy-MM-dd hh:mm:ss.zzz" для абсолютного времени и "<V> <d|h|m|s|ms|us|ns> ..." для относительного

tests/CMakeLists.txt

@@ -37,3 +37,4 @@ pip_test(piobject)
 pip_test(client_server pip_client_server)
 pip_test(io)
 pip_test(system)
+pip_test(thread)

tests/client_server/client_server_test.cpp

@@ -32,6 +32,7 @@ Client * createAndConnectClient() {
 TEST(ClientServer, OneClient) {
 	auto const loop_timeout = 1000_ms;
 	auto s                  = createServer<false, true>();
+	piMinSleep();
 	auto c                  = createAndConnectClient<TestClient<false, false>>();
 
 	waitLoop([s]() { return s->clientsCount() > 0; }, loop_timeout);
@@ -106,6 +107,7 @@ TEST(ClientServer, ManyClients) {
 	constexpr int clients_count = 20;
 	PIVector<ClientSendThread *> clients;
 	auto s = createServer<false, false, 100_KiB>();
+	piMinSleep();
 
 	piForTimes(clients_count) {
 		clients.append(new ClientSendThread());
@@ -137,7 +139,7 @@ TEST(ClientServer, ManyClients) {
 	for (const auto c: clients) {
 		c->startSend();
 	}
-	(100_ms).sleep();
+	loop_timeout.sleep();
 	EXPECT_TRUE(getClientsPings(clients) > clients_count * 2);
 	EXPECT_TRUE(getServerPongs(s) > clients_count * 2);
 	piDeleteAllAndClear(clients);
@@ -147,7 +149,7 @@ TEST(ClientServer, ManyClients) {
 }
 
 TEST(ClientServer, DynamicClients) {
-	auto const loop_timeout     = 100_ms;
+	auto const loop_timeout     = 10_ms;
 	constexpr int clients_count = 20;
 	PIVector<ClientSendThread *> clients;
 	PIMutex clients_mutex;
@@ -160,7 +162,6 @@ TEST(ClientServer, DynamicClients) {
 		clients_mutex.lock();
 		clients << c;
 		clients_mutex.unlock();
-		piCout << "new client" << clients.size();
 	};
 
 	piForTimes(clients_count) {
@@ -178,9 +179,8 @@ TEST(ClientServer, DynamicClients) {
 			piForTimes(new_cnt) {
 				spawnClient();
 			}
-			piCout << "+++++++";
 		},
-		12_Hz);
+		120_Hz);
 
 	deleteThread.start(
 		[&clients, &clients_mutex]() {
@@ -194,28 +194,20 @@ TEST(ClientServer, DynamicClients) {
 				clients_mutex.unlock();
 				if (c) {
 					delete c;
-					piCout << "remove client" << clients.size();
 				}
 			}
-			piCout << "----------";
 		},
-		13_Hz);
+		130_Hz);
 
-	(2_s).sleep();
+	(loop_timeout * clients_count).sleep();
 
 	EXPECT_GE(s->clientsCount(), 10);
 
-	piCout << "now clients" << clients.size();
-
-
 	deleteThread.stopAndWait();
 	spawnThread.stopAndWait();
 
-
-	piCout << "total clients" << clients.size();
-
 	piDeleteAllAndClear(clients);
-	waitLoop([s]() { return s->clientsCount() == 0; }, loop_timeout);
+	waitLoop([s]() { return s->clientsCount() == 0; }, loop_timeout * clients_count);
 	EXPECT_EQ(0, s->clientsCount());
 
 	delete s;

tests/thread/piprotectedvariable_test.cpp

@@ -0,0 +1,118 @@
+#include "piprotectedvariable.h"
+#include "pistring.h"
+#include "pithread.h"
+#include "piliterals_time.h"
+
+#include "gtest/gtest.h"
+#include <atomic>
+
+// Basic functionality tests
+TEST(PIProtectedVariable_Basic, BasicFunctionality) {
+	// Test basic set/get with different types
+	PIProtectedVariable<int> pvInt;
+	PIProtectedVariable<double> pvDouble;
+	PIProtectedVariable<PIString> pvString;
+
+	pvInt.set(123);
+	pvDouble.set(3.14159);
+	pvString.set(PIString("Hello, World!"));
+
+	EXPECT_EQ(pvInt.get(), 123);
+	EXPECT_DOUBLE_EQ(pvDouble.get(), 3.14159);
+	EXPECT_EQ(pvString.get(), PIString("Hello, World!"));
+
+	// Test operator=
+	pvInt = 999;
+	EXPECT_EQ(pvInt.get(), 999);
+
+	// Test getRef() with Pointer
+	struct TestStruct {
+		int x = 10;
+		int y = 20;
+		int getValue() const { return x + y; }
+	};
+
+	PIProtectedVariable<TestStruct> pvStruct;
+
+	auto ptr = pvStruct.getRef();
+	EXPECT_EQ(ptr->x, 10);
+	EXPECT_EQ(ptr->y, 20);
+	EXPECT_EQ(ptr->getValue(), 30);
+
+	// Modify through pointer
+	*ptr   = TestStruct();
+	ptr->x = 100;
+	EXPECT_EQ(pvStruct.get().x, 100);
+
+	// Test for Pointer
+	pvInt.set(42);
+	auto ptr1 = pvInt.getRef();
+	EXPECT_EQ(*ptr1, 42);
+	*ptr1 = 55;
+	EXPECT_EQ(*ptr1, 55);
+	auto ptr2 = std::move(ptr1);
+	EXPECT_EQ(*ptr2, 55);
+	*ptr2 = 100;
+	EXPECT_EQ(*ptr2, 100);
+	auto ptr3 = pvInt.getRef();
+	EXPECT_EQ(*ptr3, 100);
+	*ptr3 = 333;
+	EXPECT_EQ(*ptr3, 333);
+	EXPECT_EQ(pvInt.get(), 333);
+}
+
+// Thread safety tests
+TEST(PIProtectedVariable_ThreadSafety, ConcurrentReadWrite) {
+	PIProtectedVariable<int> pv;
+
+	std::atomic<int> writeCount(0);
+	std::atomic<int> readCount(0);
+	std::atomic<int> invalidReads(0);
+	const int NUM_ITERATIONS = 1000;
+	const int NUM_WRITERS    = 10;
+	const int NUM_READERS    = 20;
+	const int TOTAL_WRITES   = NUM_WRITERS * NUM_ITERATIONS;
+
+	// Collect thread handles for joining
+	PIVector<PIThread *> threads;
+
+	// Create writer threads
+	for (int i = 0; i < NUM_WRITERS; ++i) {
+		threads.push_back(new PIThread([&pv, &writeCount]() {
+			for (int j = 0; j < NUM_ITERATIONS; ++j) {
+				auto val = pv.getRef();
+				(*val)++;
+				auto val2 = pv.getRef();
+				writeCount++;
+				ASSERT_EQ(writeCount, *val2);
+			}
+		}));
+	}
+
+	// Create reader threads
+	for (int i = 0; i < NUM_READERS; ++i) {
+		threads.push_back(new PIThread([&pv, &invalidReads, &readCount]() {
+			for (int j = 0; j < NUM_ITERATIONS; ++j) {
+				auto val = pv.get();
+				readCount++;
+				// Value should always be in valid range [0, TOTAL_WRITES]
+				if (val < 0 || val > TOTAL_WRITES) {
+					invalidReads++;
+				}
+			}
+		}));
+	}
+
+	threads.forEach([](PIThread * & t) {t->startOnce();});
+	threads.forEach([](PIThread * & t) {t->waitForFinish(2_s);});
+	piDeleteAll(threads);
+
+	// Verify results
+	EXPECT_EQ(writeCount, TOTAL_WRITES);
+	EXPECT_EQ(readCount, NUM_READERS * NUM_ITERATIONS);
+	EXPECT_EQ(invalidReads, 0) << "All reads should return valid values in range [0, " << TOTAL_WRITES << "]";
+
+	// Final value should be TOTAL_WRITES
+	int finalVal = pv.get();
+	EXPECT_EQ(finalVal, TOTAL_WRITES);
+}