Fix Storage::File lock with killing and add tests.

Telegram/SourceFiles/base/tests_main.cpp

@@ -10,6 +10,8 @@ https://github.com/telegramdesktop/tdesktop/blob/master/LEGAL
 #include "reporters/catch_reporter_compact.hpp"
 #include <QFile>
 
+int (*TestForkedMethod)()/* = nullptr*/;
+
 namespace base {
 namespace assertion {
 
@@ -84,6 +86,8 @@ int main(int argc, const char *argv[]) {
 	for (auto i = 0; i != argc; ++i) {
 		if (argv[i] == QString("--touch") && i + 1 != argc) {
 			touchFile = QFile::decodeName(argv[++i]);
+		} else if (argv[i] == QString("--forked") && TestForkedMethod) {
+			return TestForkedMethod();
 		}
 	}
 	const char *catch_argv[] = {

Telegram/SourceFiles/storage/storage_encrypted_file.cpp

@@ -62,9 +62,7 @@ File::Result File::attemptOpenForRead(const EncryptionKey &key) {
 }
 
 File::Result File::attemptOpenForReadAppend(const EncryptionKey &key) {
-	if (!_data.open(QIODevice::ReadWrite)) {
+	if (!_lock.lock(_data, QIODevice::ReadWrite)) {
-		return Result::Failed;
-	} else if (!_lock.lock(_data)) {
 		return Result::LockFailed;
 	}
 	const auto size = _data.size();
@@ -75,9 +73,7 @@ File::Result File::attemptOpenForReadAppend(const EncryptionKey &key) {
 }
 
 File::Result File::attemptOpenForWrite(const EncryptionKey &key) {
-	if (!_data.open(QIODevice::WriteOnly)) {
+	if (!_lock.lock(_data, QIODevice::WriteOnly)) {
-		return Result::Failed;
-	} else if (!_lock.lock(_data)) {
 		return Result::LockFailed;
 	}
 	return writeHeader(key) ? Result::Success : Result::Failed;

Telegram/SourceFiles/storage/storage_encrypted_file_tests.cpp

@@ -9,6 +9,21 @@ https://github.com/telegramdesktop/tdesktop/blob/master/LEGAL
 
 #include "storage/storage_encrypted_file.h"
 
+#ifdef Q_OS_WIN
+#include "platform/win/windows_dlls.h"
+#endif // Q_OS_WIN
+
+#include <QtCore/QProcess>
+
+#include <thread>
+#ifdef Q_OS_MAC
+#include <mach-o/dyld.h>
+#elif defined Q_OS_LINUX // Q_OS_MAC
+#include <unistd.h>
+#endif // Q_OS_MAC || Q_OS_LINUX
+
+extern int (*TestForkedMethod)();
+
 const auto key = Storage::EncryptionKey(bytes::make_vector(
 	bytes::make_span("\
 abcdefgh01234567abcdefgh01234567abcdefgh01234567abcdefgh01234567\
@@ -17,10 +32,56 @@ abcdefgh01234567abcdefgh01234567abcdefgh01234567abcdefgh01234567\
 abcdefgh01234567abcdefgh01234567abcdefgh01234567abcdefgh01234567\
 ").subspan(0, Storage::EncryptionKey::kSize)));
 
-TEST_CASE("simple encrypted file", "[storage_encrypted_file]") {
+const auto name = QString("test.file");
-	const auto name = QString("simple.test");
-	const auto test = bytes::make_span("testbytetestbyte").subspan(0, 16);
 
+const auto test = bytes::make_span("testbytetestbyte").subspan(0, 16);
+
+struct ForkInit {
+	static int Method() {
+		Storage::File file;
+		const auto result = file.open(
+			name,
+			Storage::File::Mode::ReadAppend,
+			key);
+		if (result != Storage::File::Result::Success) {
+			return -1;
+		}
+
+		auto data = bytes::vector(16);
+		const auto read = file.read(data);
+		if (read != data.size()) {
+			return -1;
+		} else if (data != bytes::make_vector(test)) {
+			return -1;
+		}
+
+		const auto written = file.write(data);
+		if (written != data.size()) {
+			return -1;
+		}
+#ifdef _DEBUG
+		while (true) {
+			std::this_thread::sleep_for(std::chrono::seconds(1));
+		}
+#else // _DEBUG
+		std::this_thread::sleep_for(std::chrono::seconds(1));
+		return 0;
+#endif // _DEBUG
+	}
+	ForkInit() {
+#ifdef Q_OS_WIN
+		Platform::Dlls::start();
+#endif // Q_OS_WIN
+
+		TestForkedMethod = &ForkInit::Method;
+	}
+
+};
+
+ForkInit ForkInitializer;
+QProcess ForkProcess;
+
+TEST_CASE("simple encrypted file", "[storage_encrypted_file]") {
 	SECTION("writing file") {
 		Storage::File file;
 		const auto result = file.open(
@@ -66,5 +127,81 @@ TEST_CASE("simple encrypted file", "[storage_encrypted_file]") {
 }
 
 TEST_CASE("two process encrypted file", "[storage_encrypted_file]") {
+	SECTION("writing file") {
+		Storage::File file;
+		const auto result = file.open(
+			name,
+			Storage::File::Mode::Write,
+			key);
+		REQUIRE(result == Storage::File::Result::Success);
+
+		auto data = bytes::make_vector(test);
+		const auto written = file.write(data);
+		REQUIRE(written == data.size());
+	}
+	SECTION("access from subprocess") {
+		SECTION("start subprocess") {
+			const auto application = []() -> QString {
+#ifdef Q_OS_WIN
+				return "tests_storage.exe";
+#else // Q_OS_WIN
+				constexpr auto kMaxPath = 1024;
+				char result[kMaxPath] = { 0 };
+				uint32_t size = kMaxPath;
+#ifdef Q_OS_MAC
+				if (_NSGetExecutablePath(result, &size) == 0) {
+					return result;
+				}
+#else // Q_OS_MAC
+				auto count = readlink("/proc/self/exe", result, size);
+				if (count > 0) {
+					return result;
+				}
+#endif // Q_OS_MAC
+				return "tests_storage";
+#endif // Q_OS_WIN
+			}();
+
+			ForkProcess.start(application + " --forked");
+			const auto started = ForkProcess.waitForStarted();
+			REQUIRE(started);
+		}
+		SECTION("read subprocess result") {
+			std::this_thread::sleep_for(std::chrono::milliseconds(500));
+
+			Storage::File file;
+
+			const auto result = file.open(
+				name,
+				Storage::File::Mode::Read,
+				key);
+			REQUIRE(result == Storage::File::Result::Success);
+
+			auto data = bytes::vector(32);
+			const auto read = file.read(data);
+			REQUIRE(read == data.size());
+			REQUIRE(data == bytes::concatenate(test, test));
+		}
+		SECTION("take subprocess result") {
+			REQUIRE(ForkProcess.state() == QProcess::Running);
+
+			Storage::File file;
+
+			const auto result = file.open(
+				name,
+				Storage::File::Mode::ReadAppend,
+				key);
+			REQUIRE(result == Storage::File::Result::Success);
+
+			auto data = bytes::vector(32);
+			const auto read = file.read(data);
+			REQUIRE(read == data.size());
+			REQUIRE(data == bytes::concatenate(test, test));
+
+			const auto finished = ForkProcess.waitForFinished(0);
+			REQUIRE(finished);
+			REQUIRE(ForkProcess.state() == QProcess::NotRunning);
+		}
+	}
 
 }

Telegram/SourceFiles/storage/storage_file_lock.h

@@ -17,7 +17,7 @@ class FileLock {
 public:
 	FileLock();
 
-	bool lock(const QFile &file);
+	bool lock(QFile &file, QIODevice::OpenMode mode);
 	void unlock();
 
 	static constexpr auto kSkipBytes = size_type(4);

Telegram/SourceFiles/storage/storage_file_lock_posix.cpp

@@ -18,12 +18,17 @@ namespace Storage {
 namespace {
 
 bool KillProcess(pid_t pid) {
+	auto signal = SIGTERM;
+	auto attempts = 0;
 	while (true) {
-		const auto result = kill(pid, SIGTERM);
+		const auto result = kill(pid, signal);
 		if (result < 0) {
 			return (errno == ESRCH);
 		}
 		usleep(10000);
+		if (++attempts == 50) {
+			signal = SIGKILL;
+		}
 	}
 }
 
@@ -85,8 +90,14 @@ FileLock::Lock::~Lock() {
 
 FileLock::FileLock() = default;
 
-bool FileLock::lock(const QFile &file) {
+bool FileLock::lock(QFile &file, QIODevice::OpenMode mode) {
+	Expects(_lock == nullptr || file.isOpen());
+
 	unlock();
+	file.close();
+	if (!file.open(mode)) {
+		return false;
+	}
 	while (true) {
 		const auto result = Lock::Acquire(file);
 		if (const auto descriptor = base::get_if<Descriptor>(&result)) {
@@ -94,10 +105,10 @@ bool FileLock::lock(const QFile &file) {
 				_lock = std::make_unique<Lock>(descriptor->value);
 				return true;
 			}
-			return false;
+			break;
 		} else if (const auto pid = base::get_if<LockingPid>(&result)) {
 			if (pid->value <= 0 || !KillProcess(pid->value)) {
-				return false;
+				break;
 			}
 		}
 	}

Telegram/SourceFiles/storage/storage_file_lock_win.cpp

@@ -116,13 +116,19 @@ FileLock::Lock::~Lock() {
 
 FileLock::FileLock() = default;
 
-bool FileLock::lock(const QFile &file) {
+bool FileLock::lock(QFile &file, QIODevice::OpenMode mode) {
+	Expects(_lock == nullptr || file.isOpen());
+
+	unlock();
+	file.close();
 	do {
-		unlock();
+		if (!file.open(mode)) {
-		if (const auto descriptor = Lock::Acquire(file)) {
+			return false;
+		} else if (const auto descriptor = Lock::Acquire(file)) {
 			_lock = std::make_unique<Lock>(descriptor);
 			return true;
 		}
+		file.close();
 	} while (CloseProcesses(file.fileName()));
 
 	return false;