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 <[email protected]>
This commit is contained in:
Jérôme Duval
2024-10-19 18:39:49 +00:00
committed by waddlesplash
parent b78edf5814
commit e4a557f372
2 changed files with 40 additions and 12 deletions
+36 -10
View File
@@ -185,7 +185,7 @@ public:
void NotifyBytesWritten(size_t bytes); void NotifyBytesWritten(size_t bytes);
void NotifyEndClosed(bool writer); void NotifyEndClosed(bool writer);
void Open(int openMode); status_t Open(int openMode);
void Close(file_cookie* cookie); void Close(file_cookie* cookie);
int32 ReaderCount() const { return fReaderCount; } int32 ReaderCount() const { return fReaderCount; }
int32 WriterCount() const { return fWriterCount; } int32 WriterCount() const { return fWriterCount; }
@@ -209,7 +209,7 @@ private:
mutex fRequestLock; mutex fRequestLock;
ConditionVariable fWriteCondition; ConditionVariable fActiveCondition;
int32 fReaderCount; int32 fReaderCount;
int32 fWriterCount; int32 fWriterCount;
@@ -352,7 +352,7 @@ Inode::Inode()
fReadSelectSyncPool(NULL), fReadSelectSyncPool(NULL),
fWriteSelectSyncPool(NULL) fWriteSelectSyncPool(NULL)
{ {
fWriteCondition.Publish(this, "pipe"); fActiveCondition.Publish(this, "pipe");
mutex_init(&fRequestLock, "pipe request"); mutex_init(&fRequestLock, "pipe request");
bigtime_t time = real_time_clock(); bigtime_t time = real_time_clock();
@@ -364,7 +364,7 @@ Inode::Inode()
Inode::~Inode() Inode::~Inode()
{ {
fWriteCondition.Unpublish(); fActiveCondition.Unpublish();
mutex_destroy(&fRequestLock); mutex_destroy(&fRequestLock);
} }
@@ -566,7 +566,7 @@ Inode::NotifyBytesRead(size_t bytes)
size_t minWriteCount = request->MinimalWriteCount(); size_t minWriteCount = request->MinimalWriteCount();
if (minWriteCount > 0 && minWriteCount <= writable if (minWriteCount > 0 && minWriteCount <= writable
&& minWriteCount > writable - bytes) { && minWriteCount > writable - bytes) {
fWriteCondition.NotifyAll(); fActiveCondition.NotifyAll();
break; break;
} }
} }
@@ -620,7 +620,7 @@ Inode::NotifyEndClosed(bool writer)
} }
} else { } else {
// Last reader is gone. Wake up all writers. // Last reader is gone. Wake up all writers.
fWriteCondition.NotifyAll(); fActiveCondition.NotifyAll();
if (fWriteSelectSyncPool) if (fWriteSelectSyncPool)
notify_select_event_pool(fWriteSelectSyncPool, B_SELECT_ERROR); notify_select_event_pool(fWriteSelectSyncPool, B_SELECT_ERROR);
@@ -628,7 +628,7 @@ Inode::NotifyEndClosed(bool writer)
} }
void status_t
Inode::Open(int openMode) Inode::Open(int openMode)
{ {
MutexLocker locker(RequestLock()); MutexLocker locker(RequestLock());
@@ -639,6 +639,27 @@ Inode::Open(int openMode)
if ((openMode & O_ACCMODE) == O_RDONLY || (openMode & O_ACCMODE) == O_RDWR) if ((openMode & O_ACCMODE) == O_RDONLY || (openMode & O_ACCMODE) == O_RDWR)
fReaderCount++; 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) { if (fReaderCount > 0 && fWriterCount > 0) {
TRACE("Inode %p::Open(): fifo becomes active\n", this); TRACE("Inode %p::Open(): fifo becomes active\n", this);
fBuffer.CreateBuffer(); fBuffer.CreateBuffer();
@@ -647,8 +668,9 @@ Inode::Open(int openMode)
// notify all waiting writers that they can start // notify all waiting writers that they can start
if (fWriteSelectSyncPool) if (fWriteSelectSyncPool)
notify_select_event_pool(fWriteSelectSyncPool, B_SELECT_WRITE); 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 // Notify any still reading writers to stop
// TODO: This only works reliable if there is only one writer - we could // TODO: This only works reliable if there is only one writer - we could
// do the same thing done for the read requests. // do the same thing done for the read requests.
fWriteCondition.NotifyAll(B_FILE_ERROR); fActiveCondition.NotifyAll(B_FILE_ERROR);
} }
if (fReaderCount == 0 && fWriterCount == 0) { 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); TRACE(" open cookie = %p\n", cookie);
cookie->open_mode = openMode; cookie->open_mode = openMode;
inode->Open(openMode); status_t status = inode->Open(openMode);
if (status != B_OK) {
free(cookie);
return status;
}
*_cookie = (void*)cookie; *_cookie = (void*)cookie;
+4 -2
View File
@@ -9604,10 +9604,12 @@ _user_create_pipe(int* userFDs)
} }
// Everything looks good so far. Open two FDs for reading respectively // 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]; 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); fds[1] = open_vnode(vnode, O_WRONLY, false);
// Reset O_NONBLOCK
_kern_fcntl(fds[0], F_SETFL, 0);
FDCloser closer0(fds[0], false); FDCloser closer0(fds[0], false);
FDCloser closer1(fds[1], false); FDCloser closer1(fds[1], false);