Avoid readToEnd() on pipes.
#1394
Conversation
Attempting to read to the end of a pipe will block if the writer hasn't finished writing bytes. This can lead to a deadlock (or, at least, pathologically bad read performance) if the writer is _also_ communicating with the reader via another file descriptor or other IPC mechanism. It can also lead to difficult-to-describe issues when the pipe's kernel-side buffer is filled and the writer is blocked waiting for the reader to empty it. This PR introduces a function on `FileHandle` that reads line-by-line which gives the reader a chance to process data as it comes in rather than waiting for EOF.
grynspan
added
performance
| /// Use this function when calling C I/O interfaces such as `fputs()` on the | ||
| /// underlying C file handle. | ||
| borrowing func withUnsafeCFILEHandle<R>(_ body: (SWT_FILEHandle) throws -> R) rethrows -> R { | ||
| borrowing func withUnsafeCFILEHandle<R>(_ body: (SWT_FILEHandle) throws -> R) rethrows -> R where R: ~Copyable { |
There was a problem hiding this comment.
What's the rationale behind the R: ~Copyable in the where clause here? Is it to prevent copying of anything returned by body as a performance optimisation?
There was a problem hiding this comment.
I think that's a remnant of an earlier iteration. I'll revert it.
| if 0 != ferror(file) { | ||
| throw CError(rawValue: swt_errno()) | ||
| } | ||
| } else if let byteRead = UInt8(exactly: byteRead) { |
There was a problem hiding this comment.
This is just verifying it really is a char if it's not EOF?
There was a problem hiding this comment.
The value we get back is an int or CInt and needs to be cast to the correct type. In practice, the if let can't fail, so this code is equivalent to let byteRead = UInt8(byteRead).
| /// thrown by `isDelimiter`. | ||
| func read(until isDelimiter: (UInt8) throws -> Bool) throws -> [UInt8] { | ||
| var line = [UInt8]() | ||
| line.reserveCapacity(1024) |
There was a problem hiding this comment.
What's the reasoning behind this capacity? Seems to me you could get much longer lines for a given delimiter, so is this just based off experience?
There was a problem hiding this comment.
When I wrote my question I forgot this is for lines of the JSON event stream; now the limit makes more sense to me
There was a problem hiding this comment.
Utterly arbitrary. Gotta pick something, no way to know what.
Attempting to read to the end of a pipe will block if the writer hasn't finished writing bytes. This can lead to a deadlock (or, at least, pathologically bad read performance) if the writer is also communicating with the reader via another file descriptor or other IPC mechanism. It can also lead to difficult-to-describe issues when the pipe's kernel-side buffer is filled and the writer is blocked waiting for the reader to empty it.
This PR introduces a function on
FileHandlethat reads line-by-line which gives the reader a chance to process data as it comes in rather than waiting for EOF.Checklist: