From 31ab78ae8e10af948ec95496df0d2abf1ea631a4 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Mon, 13 Nov 2017 17:08:00 +0100
Subject: [PATCH] input/{cdio,ffmpeg,file,smbclient}: unlock the mutex during
 blocking I/O

InputStream::Read() and InputStream::Seek() are called with the mutex
locked.  That means the implementation must not block, or unlock the
mutex before calling into blocking code.

Previously, a slow CD drive could stall the whole MPD process,
including the main thread, due to this problem.

Closes #149
---
 NEWS                                          |  2 ++
 src/input/plugins/CdioParanoiaInputPlugin.cxx |  7 ++++++-
 src/input/plugins/FfmpegInputPlugin.cxx       | 15 +++++++++++++--
 src/input/plugins/FileInputPlugin.cxx         | 14 ++++++++++++--
 src/input/plugins/SmbclientInputPlugin.cxx    |  2 ++
 5 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index b89fa5cf6..c961c25db 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 ver 0.20.12 (not yet released)
 * input
+  - cdio_paranoia, ffmpeg, file, smbclient: reduce lock contention,
+    fixing lots of xrun problems
   - curl: fix seeking
 * decoder
   - ffmpeg: fix GCC 8 warning
diff --git a/src/input/plugins/CdioParanoiaInputPlugin.cxx b/src/input/plugins/CdioParanoiaInputPlugin.cxx
index 39f90dcc0..e5a63a8d6 100644
--- a/src/input/plugins/CdioParanoiaInputPlugin.cxx
+++ b/src/input/plugins/CdioParanoiaInputPlugin.cxx
@@ -270,7 +270,10 @@ CdioParanoiaInputStream::Seek(offset_type new_offset)
 	lsn_relofs = new_offset / CDIO_CD_FRAMESIZE_RAW;
 	offset = new_offset;
 
-	cdio_paranoia_seek(para, lsn_from + lsn_relofs, SEEK_SET);
+	{
+		const ScopeUnlock unlock(mutex);
+		cdio_paranoia_seek(para, lsn_from + lsn_relofs, SEEK_SET);
+	}
 }
 
 size_t
@@ -292,6 +295,8 @@ CdioParanoiaInputStream::Read(void *ptr, size_t length)
 
 		//current sector was changed ?
 		if (lsn_relofs != buffer_lsn) {
+			const ScopeUnlock unlock(mutex);
+
 			rbuf = cdio_paranoia_read(para, nullptr);
 
 			s_err = cdda_errors(drv);
diff --git a/src/input/plugins/FfmpegInputPlugin.cxx b/src/input/plugins/FfmpegInputPlugin.cxx
index 4ba9b470a..9ca16aa67 100644
--- a/src/input/plugins/FfmpegInputPlugin.cxx
+++ b/src/input/plugins/FfmpegInputPlugin.cxx
@@ -104,7 +104,13 @@ input_ffmpeg_open(const char *uri,
 size_t
 FfmpegInputStream::Read(void *ptr, size_t read_size)
 {
-	auto result = avio_read(h, (unsigned char *)ptr, read_size);
+	int result;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		result = avio_read(h, (unsigned char *)ptr, read_size);
+	}
+
 	if (result <= 0) {
 		if (result < 0)
 			throw MakeFfmpegError(result, "avio_read() failed");
@@ -126,7 +132,12 @@ FfmpegInputStream::IsEOF() noexcept
 void
 FfmpegInputStream::Seek(offset_type new_offset)
 {
-	auto result = avio_seek(h, new_offset, SEEK_SET);
+	int64_t result;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		result = avio_seek(h, new_offset, SEEK_SET);
+	}
 
 	if (result < 0)
 		throw MakeFfmpegError(result, "avio_seek() failed");
diff --git a/src/input/plugins/FileInputPlugin.cxx b/src/input/plugins/FileInputPlugin.cxx
index fc51715ad..2d2850c74 100644
--- a/src/input/plugins/FileInputPlugin.cxx
+++ b/src/input/plugins/FileInputPlugin.cxx
@@ -87,14 +87,24 @@ input_file_open(gcc_unused const char *filename,
 void
 FileInputStream::Seek(offset_type new_offset)
 {
-	reader.Seek((off_t)new_offset);
+	{
+		const ScopeUnlock unlock(mutex);
+		reader.Seek((off_t)new_offset);
+	}
+
 	offset = new_offset;
 }
 
 size_t
 FileInputStream::Read(void *ptr, size_t read_size)
 {
-	size_t nbytes = reader.Read(ptr, read_size);
+	size_t nbytes;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		nbytes = reader.Read(ptr, read_size);
+	}
+
 	offset += nbytes;
 	return nbytes;
 }
diff --git a/src/input/plugins/SmbclientInputPlugin.cxx b/src/input/plugins/SmbclientInputPlugin.cxx
index 35bb5c5b8..80d406692 100644
--- a/src/input/plugins/SmbclientInputPlugin.cxx
+++ b/src/input/plugins/SmbclientInputPlugin.cxx
@@ -128,6 +128,7 @@ SmbclientInputStream::Read(void *ptr, size_t read_size)
 	ssize_t nbytes;
 
 	{
+		const ScopeUnlock unlock(mutex);
 		const std::lock_guard<Mutex> lock(smbclient_mutex);
 		nbytes = smbc_read(fd, ptr, read_size);
 	}
@@ -145,6 +146,7 @@ SmbclientInputStream::Seek(offset_type new_offset)
 	off_t result;
 
 	{
+		const ScopeUnlock unlock(mutex);
 		const std::lock_guard<Mutex> lock(smbclient_mutex);
 		result = smbc_lseek(fd, new_offset, SEEK_SET);
 	}