From e4a557f372b21b348e0c6a2ae7157c1b73e0d738 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Duval?= <jerome.duval@gmail.com>
Date: Fri, 18 Oct 2024 19:05:19 +0200
Subject: [PATCH] kernel/fifo: honor O_NONBLOCK on open() for FIFOs

according to POSIX:
- O_NONBLOCK set:
	* read-only open() shall return asap
	* write-only open() shall return an error if no reader.
- O_NONBLOCK clear:
	* read-only open() shall block until a writer opens.
	* write-only open() shall block until a reader opens.

for FIFOs used for pipes, open with O_NONBLOCK then reset O_NONBLOCK.

can be tested with:
bash -norc -c '/bin/printf "test" > >(/bin/tee -a trace.log) 2>&1'

Change-Id: I3cb70810fec50f643b5e6e84dbdeb0a16961936a
Reviewed-on: https://review.haiku-os.org/c/haiku/+/8469
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
---
 src/system/kernel/fs/fifo.cpp | 46 +++++++++++++++++++++++++++--------
 src/system/kernel/fs/vfs.cpp  |  6 +++--
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/system/kernel/fs/fifo.cpp b/src/system/kernel/fs/fifo.cpp
index 02441b8456..d557df941c 100644
--- a/src/system/kernel/fs/fifo.cpp
+++ b/src/system/kernel/fs/fifo.cpp
@@ -185,7 +185,7 @@ public:
 			void				NotifyBytesWritten(size_t bytes);
 			void				NotifyEndClosed(bool writer);
 
-			void				Open(int openMode);
+			status_t			Open(int openMode);
 			void				Close(file_cookie* cookie);
 			int32				ReaderCount() const { return fReaderCount; }
 			int32				WriterCount() const { return fWriterCount; }
@@ -209,7 +209,7 @@ private:
 
 			mutex				fRequestLock;
 
-			ConditionVariable	fWriteCondition;
+			ConditionVariable	fActiveCondition;
 
 			int32				fReaderCount;
 			int32				fWriterCount;
@@ -352,7 +352,7 @@ Inode::Inode()
 	fReadSelectSyncPool(NULL),
 	fWriteSelectSyncPool(NULL)
 {
-	fWriteCondition.Publish(this, "pipe");
+	fActiveCondition.Publish(this, "pipe");
 	mutex_init(&fRequestLock, "pipe request");
 
 	bigtime_t time = real_time_clock();
@@ -364,7 +364,7 @@ Inode::Inode()
 
 Inode::~Inode()
 {
-	fWriteCondition.Unpublish();
+	fActiveCondition.Unpublish();
 	mutex_destroy(&fRequestLock);
 }
 
@@ -566,7 +566,7 @@ Inode::NotifyBytesRead(size_t bytes)
 			size_t minWriteCount = request->MinimalWriteCount();
 			if (minWriteCount > 0 && minWriteCount <= writable
 					&& minWriteCount > writable - bytes) {
-				fWriteCondition.NotifyAll();
+				fActiveCondition.NotifyAll();
 				break;
 			}
 		}
@@ -620,7 +620,7 @@ Inode::NotifyEndClosed(bool writer)
 		}
 	} else {
 		// Last reader is gone. Wake up all writers.
-		fWriteCondition.NotifyAll();
+		fActiveCondition.NotifyAll();
 
 		if (fWriteSelectSyncPool)
 			notify_select_event_pool(fWriteSelectSyncPool, B_SELECT_ERROR);
@@ -628,7 +628,7 @@ Inode::NotifyEndClosed(bool writer)
 }
 
 
-void
+status_t
 Inode::Open(int openMode)
 {
 	MutexLocker locker(RequestLock());
@@ -639,6 +639,27 @@ Inode::Open(int openMode)
 	if ((openMode & O_ACCMODE) == O_RDONLY || (openMode & O_ACCMODE) == O_RDWR)
 		fReaderCount++;
 
+	bool shouldWait = false;
+	if ((openMode & O_ACCMODE) == O_WRONLY && fReaderCount == 0) {
+		if ((openMode & O_NONBLOCK) != 0)
+			return ENXIO;
+		shouldWait = true;
+	}
+	if ((openMode & O_ACCMODE) == O_RDONLY && fWriterCount == 0
+		&& (openMode & O_NONBLOCK) == 0) {
+		shouldWait = true;
+	}
+	if (shouldWait) {
+		// prepare for waiting for the condition variable.
+		ConditionVariableEntry waitEntry;
+		fActiveCondition.Add(&waitEntry);
+		locker.Unlock();
+		status_t status = waitEntry.Wait(B_CAN_INTERRUPT);
+		if (status != B_OK)
+			return status;
+		locker.Lock();
+	}
+
 	if (fReaderCount > 0 && fWriterCount > 0) {
 		TRACE("Inode %p::Open(): fifo becomes active\n", this);
 		fBuffer.CreateBuffer();
@@ -647,8 +668,9 @@ Inode::Open(int openMode)
 		// notify all waiting writers that they can start
 		if (fWriteSelectSyncPool)
 			notify_select_event_pool(fWriteSelectSyncPool, B_SELECT_WRITE);
-		fWriteCondition.NotifyAll();
+		fActiveCondition.NotifyAll();
 	}
+	return B_OK;
 }
 
 
@@ -682,7 +704,7 @@ Inode::Close(file_cookie* cookie)
 		// Notify any still reading writers to stop
 		// TODO: This only works reliable if there is only one writer - we could
 		// do the same thing done for the read requests.
-		fWriteCondition.NotifyAll(B_FILE_ERROR);
+		fActiveCondition.NotifyAll(B_FILE_ERROR);
 	}
 
 	if (fReaderCount == 0 && fWriterCount == 0) {
@@ -888,7 +910,11 @@ fifo_open(fs_volume* _volume, fs_vnode* _node, int openMode,
 
 	TRACE("  open cookie = %p\n", cookie);
 	cookie->open_mode = openMode;
-	inode->Open(openMode);
+	status_t status = inode->Open(openMode);
+	if (status != B_OK) {
+		free(cookie);
+		return status;
+	}
 
 	*_cookie = (void*)cookie;
 
diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index fa7a0ed28e..43dbab9601 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -9604,10 +9604,12 @@ _user_create_pipe(int* userFDs)
 	}
 
 	// Everything looks good so far. Open two FDs for reading respectively
-	// writing.
+	// writing, O_NONBLOCK to avoid blocking on open with O_RDONLY
 	int fds[2];
-	fds[0] = open_vnode(vnode, O_RDONLY, false);
+	fds[0] = open_vnode(vnode, O_RDONLY | O_NONBLOCK, false);
 	fds[1] = open_vnode(vnode, O_WRONLY, false);
+	// Reset O_NONBLOCK
+	_kern_fcntl(fds[0], F_SETFL, 0);
 
 	FDCloser closer0(fds[0], false);
 	FDCloser closer1(fds[1], false);