libc: readv should only block once

[syborg64]

The bsd spec for readv states that readv can return less bytes than requested, just like read.

The bug happens when a subsequent read() call blocks for more input when the stream is currently exhausted.
The readv() call remains blocked and doesn't return the previously read bytes.

The problem also assumedly happens for writev as they share an implementation.

possible fix: insert poll before each subsequent read()/write() call to release in case the stream is blocked. I'm linking a commit that does that.

---

Somewhat unrelated, probable bug found in the same function:
The Mutex refactoring changed the behavior of the mutex protecting both readv and writev. It used to be shared between the different implemation with a global static Lock but was replaced with a local static Mutex.
The change is significant because the templated function is instantiated for both Read and Write and both instances of the mutex are now unrelated and no longer protecting from reading and writing at the same time.

This change is annotated with a fixme about using a pthread_mutex instead because of blocking operations.

---

Questions for the qualified:

• are there any concerns with using poll here ?
• should I open an other issue for the mutex ? what should be done about it ?

[chelmuth]

@cproc do you have any stance on this suggestion?

[cproc]

Does commit a0949b2 fix the problem too? If one of the read() or write() calls transferred less than requested the readv() or writev() function just returns.

Regarding the mutex it might make sense to open another issue because it might not be clear which cases actually need protection. Is it only readv() and writev() or do we need to prevent parallel execution of all combinations of read(), write(), pread(), pwrite(), readv() and writev()?

[syborg64]

Yep! a0949b2 fixes it, and is indeed a lot more elegant than polling.

cproc, you do seem to be the originator of the lock guard. I wouldn't be surprised if it was an attempt to make it behave more atomically.

[cproc]

cproc, you do seem to be the originator of the lock guard. I wouldn't be surprised if it was an attempt to make it behave more atomically.

Yes, I added the global static lock to pread()/pwrite() at first and then to readv()/writev(). But unfortunately I can't remember if there was an actual situation where it was needed or if it was just an easy to add precaution.

pread()/pwrite() uses a file descriptor mutex now and readv()/writev() has the FIXME comment about pthread_mutex which probably applies to the file descriptor mutex as well? So maybe we should replace the file descriptor mutex with a file descriptor pthread_mutex and use it in readv()/writev() too? That would cover at least all combinations of pread(), pwrite(), readv() and writev(), but would not prevent a parallel read() or write() call in between the multiple calls in readv()/writev().

[chelmuth]

I merged the fix to staging.

The atomicity constraints of the read/write operations became not clear to me from this short discussion. Therefore, I suggest to defer any changes to after a clear statement in this regard has been developed.