wahern / cqueues

Continuation Queues: Embeddable asynchronous networking, threading, and notification framework for Lua on Unix.
http://25thandclement.com/~william/projects/cqueues.html
MIT License
250 stars 37 forks source link

eof set on closed socket even if data left. #127

Open daurnimator opened 8 years ago

daurnimator commented 8 years ago
local cs = require "cqueues.socket"
local c, s = cs.pair()
assert(c:xwrite("foo\nbar\n", "n"))
c:close()
assert(s:read() == "foo")
print(s:eof("r"))

true

Is this intentional? If so, I can work around if via :pending(); but I don't think that's correct?

daurnimator commented 8 years ago

I was imagining :eof("r") would act more like lso_nomore: returning true if any data can be read.

While searching for the best place to fix this issue, I think I spotted a bug in lso_nomore. fix:

diff --git a/src/socket.c b/src/socket.c
index 31e06b7..046e50b 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -1646,7 +1646,7 @@ static lso_error_t lso_fill(struct luasocket *S, size_t limit) {

 static _Bool lso_nomore(struct luasocket *S, size_t limit) {
-       return S->ibuf.eof || S->ibuf.eom || fifo_rlen(&S->ibuf.fifo) >= limit;
+       return S->ibuf.eom || (S->ibuf.eof && fifo_rlen(&S->ibuf.fifo) < limit);
 } /* lso_nomore() */
wahern commented 8 years ago

I think the bug (such as it is) is elsewhere, perhaps in the :eof method.

The logic in lso_nomore looks okay. The purpose of lso_nomore is to determine whether an operation should fill the request out of the buffer or wait for more data. We fill the request when we've reached EOM, EOF, or if we've exceeded the requestor's limit on the amount of data it wishes to buffer (#bytes >= limit). In the latter case we want to return true regardless of EOF state.

Think of it like fgets in C--if a newline hasn't been reached within the size of the provided buffer, fgets fills the buffer sans-newline and returns success rather than waiting for more data.

If an application doesn't want this behavior they can issue :setmaxline(math.huge), and suffer any repercussions of a peer sending an infinitely long string.

daurnimator commented 8 years ago

Agreed. I guess the fix is to add && fifo_rlen(...) == 0 to the eof("r") check.