discontinue readavailable()

[vtjnash]

This is really hard function to use correctly, since the only correct usage of this function is in a stream reader:

buf = Uint8[]
while !eof(s)
  bytes = readavailable(s)
  append!(buf, s)
  if enough(buf)
     f(buf)
     empty!(buf)
  end
end
f(buf)

However, essentially all of the code I see posted to blogs and mailing lists to use this function are incorrectly defined, and subject to race conditions and the badly-defined behavior of this function (that and probably will stop working as soon as you try to bundle it into your application and output a decent amount of information, or forget that stdout is buffered, or run into a bit about latency)

I have seen users resort to it in the following situation, when users can't understand why readall hangs:

(outRead, outWrite) = redirect_stdout()

print("Test")
print("ing")

close(outWrite)

data = readavailable(outRead)

close(outRead)

(copied from: http://thenewphalls.wordpress.com/2014/03/21/capturing-output-in-julia/)

however, the answer to their quandary is simple: outWrite is only one of the handles to the stdout. until you call redirect_stdout again, julia still has a second handle open (on fd 2) for use by c functions

related: #7022

[ihnorton]

(@vtjnash did you intend to pull in helpdb changes?)

[vtjnash]

(yes, i'm not sure why it is so big though, apparently it wanted to insert a lot of blank lines?)

[vtjnash]

this is a separate bugfix for the readall function, which needs to be committed separately if Jeff refuses to merge this pull request

[JeffBezanson]

+1
I greatly dislike readavailable; it has essentially no meaning at all.

If you have a separate bugfix, it belongs on master, ideally with a test case, not bundled with whatever PR you are working on. That is simply sound development practice, and has nothing to do with my willingness to merge PRs. Coupling the decision to fix a bug to the decision to deprecate some other function is insane. Separating them also allows others to selectively revert or run bisect more effectively.

You seem to have one of the unlucky versions of sphinx that inserts extra unwanted newlines. In that case, please don't regenerate the helpdb. Somebody else can do it.

[JeffBezanson]

This warning is a bit too passive-aggressive. Needs a rewrite.

[vtjnash]

how about:
"readavailable is discontinued. please choose from the alternative I/O functions, such as readall(), readbytes(), nb_available(), and !eof(), depending on your intended usage"

[vtjnash]

ideally with a test case

the fixes to the test files revealed the bug in readall, so it does test it. i'm not working from master right now, but I can repeat the fix on master if you aren't going to just merge this anyways

[JeffBezanson]

Let me repeat: whether the bug fix is a separate change is unrelated to whether I merge this PR.

[JeffBezanson]

The modified repl test passes for me on master, without the readcb change.

[vtjnash]

The repl test might pass, but it is dependent upon order-of-execution. Since we want to consider EOF on TTY equivalent to close, it needs to emit the close notify event to be certain any waiting conditions get notified.

[JeffBezanson]

Isn't the notify needed for streams other than TTYs as well though?

[vtjnash]

what is there to notify? other streams don't have a EOF event, and they will eventually get a close event after the stream is closed

[JeffBezanson]

Ah yes, I see there is a try notify(uv.readnotify) end in _uv_hook_close.

[vtjnash]

yes. that's a bit of a hack, because wait can't wait on multiple Conditions (hint, hint)

[astrieanna]

Are you aware of any stream readers/parsers other than HttpParser, in Julia? I guess this is probably not a common enough task to warrant a (confusing) function in Base.

Considering that people are apparently using readavailable as a work around when readall doesn't work, is there documentation anywhere explaining why that happens? (Or are you hoping that they'll post issues or email the mailing list when they get stuck, once you remove readavailable?)

[JeffBezanson]

Does it work to read fixed-size chunks and then look for short reads to detect end of stream?

[stevengj]

We use readavailable as a stream reader in IJulia. I'm not sure what @vtjnash is proposing as an alternative here.

I suppose we could block on reading one byte, and then call nb_available to see if anything more is available and do a readbytes with that many bytes? But readavailable is a lot more convenient.

[vtjnash]

I would use:
while !eof(s)
readbytes(s,nb_available(s))
end

Or maybe, takebuf_array?

This formulation also avoids the busy-wait lockup if s is ever closed that exists in the current code.

[stevengj]

@vtjnash, that seems like it will be a spinloop (since nb_available will mostly return zero, readbytes will return immediately), which is not acceptable. We need something that will block until there is actually something available to read (but doesn't wait for the stream to close, unlike readall).

(The stream is never closed in IJulia until the process completes, so that's not an issue.)

[JeffBezanson]

I'll repeat my question:

Does it work to read fixed-size chunks and then look for short reads to detect end of stream?

[stevengj]

@JeffBezanson, no, it doesn't work to read fixed-size chunks, unless the chunk size is 1 byte (which is too small to be efficient). Say I am reading chunks of 100 bytes, and the user prints 99 bytes to stdout. They will be waiting a long time for their output to show up in IJulia, because the I/O thread will be blocked until another byte is written.

[vtjnash]

eof blocks until it can determine if you have data, or eof. If you had wanted to do fixed size reads, you could combine the eof test with wait_nb

[JeffBezanson]

How do people usually do this? I'm guessing maybe readline, effectively using line buffering?

Anyway, since a couple packages seem pretty happy with this function I lean towards moving this change out of 0.3.

[astrieanna]

HttpParser really shouldn't use line buffering. Besides potential performance problems, some requests don't have new lines at the end.

[JeffBezanson]

Yes, line buffering would only work for stdout in IJulia.

libuv works by invoking a callback and telling you how many bytes it has. readavailable fits this kind of interface. However you can get the same effect by checking !eof and using nb_available.

[astrieanna]

You can use print to print things without newlines. Would that cause a problem for IJulia? (i.e. I print something with out a final new line, and am left waiting for my output until I decide to run something else?)

[JeffBezanson]

Yes, but for printing text to stdout people are somewhat used to line buffering. Anyway removing this function is not a huge advantage so I'm moving this issue to 0.4.

[stevengj]

I don't think it's acceptable to delay output until a newline occurs in IJulia.

However, if eof(s) blocks until something is available to read as @vtjnash suggests, then that approach sounds like it should be fine.

Still, I tend to agree that this should be a 0.4 change.

[JeffBezanson]

eof does currently block until data is available.

[vtjnash]

I should also probably point out that since readavailable is unicode-unaware, it may sometimes throw invalid string exceptions on valid strings, if a utf8 characters happened to overlap with the data discretization boundaries of the internal implementation.

[JeffBezanson]

It should probably return a byte array anyway.
On Jul 3, 2014 7:11 PM, "Jameson Nash" [email protected] wrote:

I should also probably point out that since readavailable is
unicode-unaware, it may sometimes throw invalid string exceptions on valid
strings, if a utf8 characters happened to overlap with the data
discretization boundaries of the internal implementation.

—
Reply to this email directly or view it on GitHub
#7478 (comment).

[JeffBezanson]

We will keep this function but it should return a byte array instead of a string.