Skip to content

Keep isFinished() false on StreamLog until stdin is closed#14

Open
jonscheiding wants to merge 1 commit into
acaudwell:masterfrom
jonscheiding:master
Open

Keep isFinished() false on StreamLog until stdin is closed#14
jonscheiding wants to merge 1 commit into
acaudwell:masterfrom
jonscheiding:master

Conversation

@jonscheiding
Copy link
Copy Markdown

The isFinished() method on StreamLog returns true when the stream is empty. Attempting to modify it so that (at least on POSIX platforms) it can only return true when the stream actually closes.

@acaudwell
Copy link
Copy Markdown
Owner

Hi. I tried it out and it seemed to work, I made a few comments.

@acaudwell
Copy link
Copy Markdown
Owner

Thought more about this.

Without a Windows implementation of isPipeOpen() I think the change to Gource might break streaming on Windows (re your tail example).

I wonder if there are other poll() statuses to make sure the pipe is valid.

Changing the isFinished() might have other unexpected side effects elsewhere.

Exposes a non-blocking check for whether the stdin pipe is still
connected, so callers can distinguish a stream that's transiently
empty from one whose peer has closed the pipe.

POSIX uses poll(POLLIN) and treats POLLHUP/POLLERR/POLLNVAL as
closed; Windows uses PeekNamedPipe (matching getNextLine).

isFinished() is intentionally left unchanged.
@jonscheiding
Copy link
Copy Markdown
Author

@acaudwell Revisiting this - thanks for the feedback and patience!

I reworked this to be less invasive.

  • instead of changing isFinished() I added a separate isOpen() to capture the hangup situation
  • handling POLLERR/POLLNVAL along with POLLHUP
  • Windows implementation with PeekNamedPipe

If you merge this, I'll resubmit my change in acaudwell/Gource to make use of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants