From 4cd897baddcfaefda33f7ec9628f2a2f1a56edf5 Mon Sep 17 00:00:00 2001
From: Kyle Schwarz <kschwarz@intrepidcs.com>
Date: Fri, 14 Aug 2020 16:57:52 -0400
Subject: [PATCH] Fix deadlock with Driver::write

Use a spin lock to recheck the queue size until it has room to push.
---
 communication/driver.cpp              | 4 ++--
 include/icsneo/communication/driver.h | 6 ------
 platform/posix/ftdi.cpp               | 1 -
 platform/posix/stm32.cpp              | 1 -
 platform/windows/pcap.cpp             | 1 -
 platform/windows/vcp.cpp              | 2 --
 6 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/communication/driver.cpp b/communication/driver.cpp
index 0a050c24..64aecd10 100644
--- a/communication/driver.cpp
+++ b/communication/driver.cpp
@@ -45,9 +45,9 @@ bool Driver::write(const std::vector<uint8_t>& bytes) {
 	}
 
 	if(writeBlocks) {
-		std::unique_lock<std::mutex> lk(writeMutex);
 		if(writeQueue.size_approx() > writeQueueSize)
-			writeCV.wait(lk);
+			while(writeQueue.size_approx() > (writeQueueSize * 3 / 4))
+				std::this_thread::sleep_for(std::chrono::milliseconds(10));
 	} else {
 		if(writeQueue.size_approx() > writeQueueSize) {
 			report(APIEvent::Type::TransmitBufferFull, APIEvent::Severity::Error);
diff --git a/include/icsneo/communication/driver.h b/include/icsneo/communication/driver.h
index 747ffc3d..e384c1d6 100644
--- a/include/icsneo/communication/driver.h
+++ b/include/icsneo/communication/driver.h
@@ -22,10 +22,6 @@ class Driver {
 	virtual bool read(std::vector<uint8_t>& bytes, size_t limit = 0);
 	virtual bool readWait(std::vector<uint8_t>& bytes, std::chrono::milliseconds timeout = std::chrono::milliseconds(100), size_t limit = 0);
 	virtual bool write(const std::vector<uint8_t>& bytes);
-	inline void onWrite() {
-		if(writeQueue.size_approx() < (writeQueueSize * 3/4))
-			writeCV.notify_one();
-	}
 
 	device_eventhandler_t report;
 
@@ -47,8 +43,6 @@ class Driver {
 	virtual void writeTask() = 0;
 	moodycamel::BlockingConcurrentQueue<uint8_t> readQueue;
 	moodycamel::BlockingConcurrentQueue<WriteOperation> writeQueue;
-	std::mutex writeMutex;
-	std::condition_variable writeCV;
 	std::thread readThread, writeThread;
 	std::atomic<bool> closing{false};
 };
diff --git a/platform/posix/ftdi.cpp b/platform/posix/ftdi.cpp
index 4499710a..c1979473 100644
--- a/platform/posix/ftdi.cpp
+++ b/platform/posix/ftdi.cpp
@@ -199,6 +199,5 @@ void FTDI::writeTask() {
 			continue;
 
 		ftdi.write(writeOp.bytes.data(), (int)writeOp.bytes.size());
-		onWrite();
 	}
 }
\ No newline at end of file
diff --git a/platform/posix/stm32.cpp b/platform/posix/stm32.cpp
index 159fcebf..bb5b32a0 100644
--- a/platform/posix/stm32.cpp
+++ b/platform/posix/stm32.cpp
@@ -134,6 +134,5 @@ void STM32::writeTask() {
 		ssize_t actualWritten = ::write(fd, writeOp.bytes.data(), writeSize);
 		if(actualWritten != writeSize)
 			report(APIEvent::Type::FailedToWrite, APIEvent::Severity::Error);
-		onWrite();
 	}
 }
\ No newline at end of file
diff --git a/platform/windows/pcap.cpp b/platform/windows/pcap.cpp
index dd9349ce..40fc31f2 100644
--- a/platform/windows/pcap.cpp
+++ b/platform/windows/pcap.cpp
@@ -297,7 +297,6 @@ void PCAP::writeTask() {
 		auto bs = sendPacket.getBytestream();
 		if(!closing)
 			pcap.sendpacket(interface.fp, bs.data(), (int)bs.size());
-		onWrite();
 		// TODO Handle packet send errors
 	}
 }
diff --git a/platform/windows/vcp.cpp b/platform/windows/vcp.cpp
index 24b9dcce..2b616fcf 100644
--- a/platform/windows/vcp.cpp
+++ b/platform/windows/vcp.cpp
@@ -401,8 +401,6 @@ void VCP::writeTask() {
 				bytesWritten = 0;
 				if(WriteFile(handle, writeOp.bytes.data(), (DWORD)writeOp.bytes.size(), nullptr, &overlappedWrite))
 					continue;
-
-				onWrite();
 				
 				auto winerr = GetLastError();
 				if(winerr == ERROR_IO_PENDING) {