Re: git: 9ac6eda6c6a3 - main - pipe: try to skip locking the pipe if a non-blocking fd is used
Date: Wed, 17 Aug 2022 16:14:32 UTC
On Wed, Aug 17, 2022 at 09:04:02AM -0700, John Baldwin wrote: J> On 8/17/22 8:39 AM, Gleb Smirnoff wrote: J> > On Wed, Aug 17, 2022 at 08:34:22AM -0700, Gleb Smirnoff wrote: J> > T> This also breaks API. Although not explicitly narrated by POSIX or our J> > T> documentation, but there is a strong contract that, if application had J> > T> received a non-blocking fd from event dispatcher (kevent, poll, select) J> > T> as writable, then it is possible to write thre non-zero bytes with next J> > T> syscall. J> > J> > Actually for poll(2) and kevent(2) you can almost read this contract between J> > the lines: J> > J> > POLLWRNORM Normal data may be written without blocking. J> > J> > EVFILT_WRITE Takes a descriptor as the identifier, and returns J> > whenever it is possible to write to the descriptor. J> > For sockets, pipes and fifos, data will contain the J> > amount of space remaining in the write buffer. The J> > filter will set EV_EOF when the reader disconnects, J> > and for the fifo case, this will be cleared when a J> > new reader connects. Note that this filter is not J> > supported for vnodes. J> > J> > So with this change it may happen that previous return of poll or kevent J> > returned incorrect information. J> J> While I largely agree with your sentiment, I think the actual contract is a J> bit weaker than you stated and something more like: J> J> "If a non-blocking fd becomes writable and the event is published, at least J> one thread will be able to write without blocking." J> J> That is, the contract you stated is true when there is a single writer, but J> with multiple writers it is not true. You might get a kevent back that says J> you have X bytes of room to write, yet be able to write less than X because J> of concurrent writes in other threads. Even this contract is broken, as EAGAIN can be returned to a single writer, while a reader keeps the pipe locked in the kernel. -- Gleb Smirnoff