From 19ea38fc729adf355bf6710005fd75143a9ab2dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= <sludwig@rejectedsoftware.com>
Date: Sun, 16 Jun 2019 22:30:34 +0200
Subject: [PATCH 1/2] Add test for #157.

---
 tests/issue-157-consume-closed-channel.d | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 tests/issue-157-consume-closed-channel.d

diff --git a/tests/issue-157-consume-closed-channel.d b/tests/issue-157-consume-closed-channel.d
new file mode 100644
index 0000000..8c0dfb9
--- /dev/null
+++ b/tests/issue-157-consume-closed-channel.d
@@ -0,0 +1,29 @@
+/+ dub.sdl:
+	name "tests"
+	dependency "vibe-core" path=".."
++/
+module tests;
+
+import vibe.core.channel;
+import vibe.core.core;
+import core.time;
+
+void main()
+{
+	auto ch = createChannel!int();
+
+	auto p = runTask({
+		sleep(1.seconds);
+		ch.close();
+	});
+
+	auto c = runTask({
+		while (!ch.empty) {
+			try ch.consumeOne();
+			catch (Exception e) assert(false, e.msg);
+		}
+	});
+
+	p.join();
+	c.join();
+}

From 37ad77c7015a7e2f55249454f6e607414ad5fbbb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= <sludwig@rejectedsoftware.com>
Date: Sun, 16 Jun 2019 22:33:15 +0200
Subject: [PATCH 2/2] Let empty block to make sure the following consumeOne
 succeeds.

This change ensures that a return value of false guarantees the next call to consumeOne to succeed, meaning that the combination of empty/consumeOne is sound in a single-consumer scenario. This also updates the documentation to stress that tryConsumeOne is still the preferred API.
---
 source/vibe/core/channel.d | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/source/vibe/core/channel.d b/source/vibe/core/channel.d
index e6741f8..10f389b 100644
--- a/source/vibe/core/channel.d
+++ b/source/vibe/core/channel.d
@@ -41,7 +41,7 @@ struct Channel(T, size_t buffer_size = 100) {
 
 	private shared ChannelImpl!(T, buffer_size) m_impl;
 
-	/** Determines whether there is more data to read.
+	/** Determines whether there is more data to read in a single-reader scenario.
 
 		This property is empty $(I iff) no more elements are in the internal
 		buffer and `close()` has been called. Once the channel is empty,
@@ -49,8 +49,8 @@ struct Channel(T, size_t buffer_size = 100) {
 		exception.
 
 		Note that relying on the return value to determine whether another
-		element can be read is only safe in a single-reader scenario. Use
-		`tryConsumeOne` in a multiple-reader scenario instead.
+		element can be read is only safe in a single-reader scenario. It is
+		generally recommended to use `tryConsumeOne` instead.
 	*/
 	@property bool empty() { return m_impl.empty; }
 	/// ditto
@@ -78,6 +78,10 @@ struct Channel(T, size_t buffer_size = 100) {
 
 		This function will block if no elements are available. If the `empty`
 		property is `true`, an exception will be thrown.
+
+		Note that it is recommended to use `tryConsumeOne` instead of a
+		combination of `empty` and `consumeOne` due to being more efficient and
+		also being reliable in a multiple-reader scenario.
 	*/
 	T consumeOne() { return m_impl.consumeOne(); }
 	/// ditto
@@ -143,6 +147,12 @@ private final class ChannelImpl(T, size_t buffer_size) {
 			scope (exit) m_mutex.unlock_nothrow();
 
 			auto thisus = () @trusted { return cast(ChannelImpl)this; } ();
+
+			// ensure that in a single-reader scenario !empty guarantees a
+			// successful call to consumeOne
+			while (!thisus.m_closed && thisus.m_items.empty)
+				thisus.m_condition.wait();
+
 			return thisus.m_closed && thisus.m_items.empty;
 		}
 	}