Fix deadlock with Driver::write

Use a spin lock to recheck the queue size until it has room to push.
intrepidcs · Aug 14, 2020 · 4cd897b · tysonite · Sep 1, 2020 · kschwarz-intrepidcs
1 parent afda617
commit 4cd897b
Show file tree

Hide file tree

Showing 6 changed files with 2 additions and 13 deletions.
diff --git 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
@@ -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
@@ -199,6 +199,5 @@ void FTDI::writeTask() {
 			continue;
 
 		ftdi.write(writeOp.bytes.data(), (int)writeOp.bytes.size());
-		onWrite();
 	}
 }
diff --git 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();
 	}
 }
diff --git 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
@@ -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) {