Non-blocking handling in gzread

[drbitboy]

Caveat

I am new to zlib development; perhaps this has been brought up before and found to be insoluble; if so, please forgive me and feel free to delete this issue.

Background

The functions gzread/gzwrite are a sweet pair of near-analogs for read(2)/write(2), respectively (I was trying to re-invent their wheels over the past several weeks until I stumbled onto them ;-).

I am hoping to compress a stream of multiple XML entities (INDI protocol; see github.com/magao-x/MagAOX) with zlib, and gzread/gzwrite look like a nice way to do that since they handle all the intermediate buffering. The INDI server handles multiple connections, using select(2) and non-blocking I/O.

Issue

Of course gzread works as designed, but it does depend on the compressed stream not being non-blocking, so the read(2) will block when expected data are not yet available, so a return value of 0 can be interpreted as the other end closing the file descriptor i.e. EOF.

If the compressed stream is non-blocking, then a read(2) could return a status code of -1* with errno equal to EAGAIN or EWOULDBLOCK, for the case where the next data are not yet available e.g. TCP socket, which is normal, which would be interpreted by the current code develop//ceadaf2/gzread.c/line27 as an abnormal problem and pass the -1 return up the call chain, which would stop the decompression.

* N.B. this should not be possible if the stream is blocking, so the proposed PR does not affect gzread in that case.

PR

I will make a pull request from drbitboy/zlib-drbitboy.git, branch develop-20240718-drbitboy to madler/zlib branch develop. It's a very small change at this point, but I think it is only the initial step in making what I want to do work, so even if it makes sense don't merge it yet.

The biggest concern here would be if my eventual PR breaks existing applications; the minor change to this point should not do that, but who knows what else will be necessary; in that case I think special-purpose routines in gzread.c to handle non-blocking I/O would be the best path.

Best regards,

Brian T. Carcich
Latchmoor Services, INC.

[drbitboy]

PR is #993

[drbitboy]

PR is #993

Seems to work now (drbitboy@6e8ddfd); needs a little cleaning up (comments), and I plan to write a test for it.

Let me know if there is anything else you want to see before starting a formal review.

[drbitboy]

Test added for the new code; PR should be ready to merge. lmk, thanks!

[drbitboy]

I have used these changes to add compression to the MagAOX repository; see PR drbitboy/MagAOX#8.

[madler]

I am coming back to this to understand the use case. I may want to implement it differently.

How would an application expect to use gzread() for non-blocking input? Should gzread() return an error like read() does if it has no data to return? Or are you expecting it to return zero? And in either case, how would you expect the application to detemine that either the error was due to no data or that the zero return is not EOF, and so to continue calling gzread()? Through some other call?

If gzread() returns some data (so there is no error or zero indication), but in the process it hit an EAGAIN, does the application want to know that?

How should the other functions work with non-blocking input, i.e. gzfread(), gzgetc(), gzgets()?

Should there be an option for gzopen() or gzdopen() request or set O_NONBLOCK?

[drbitboy]

Hi Mark,

As a general comment, I have my own fork for the one application where I used this functionality, so what you do with the repo is completely your call. I'll try to help you understand what is happening and what my goals were, but you are zlib's BDFL.

The genesis of this PR is that gzread was designed, and functions, as a drop-in replacement for read(2) for FDs not opened non-blocking. This PR attempts to extend that drop-in quality for FDs that are opened non-blocking.

How would an application expect to use gzread() for non-blocking input?

Like this and this, i.e. as much as possible, exactly as for read(2).

The main justification for how I did this is that, with the main zlib branch on an FD opened non-blocking e.g. on a network stream, whenever a read(2) would block, that causes a collapse of the entire gz stack for that FD, e.g. the FD is closed. So on the surface this change does not make anything worse.

I suspect that most applications running decompression via zlib do not open their FDs non-blocking (if for no other reason than that it does not work with the main zlib branch). Such applications will be unaffected by, and will not notice, the change, because the read(2) under the gzread will simply block and not return until data are available.

That said, there may be existing applications "out there, somewhere" that relies on that behavior, and using my code would cause that application to fail (e.g. because it leaves FD(s) open, so the application may experience FD exhaustion eventually). I would not want to risk the internet falling over because @drbitboy came up with this hack to add non-blocking functionality to gzread.

Should gzread() return an error like read() does if it has no data to return?

Yes, again, just as read(2) does, i.e. return -1 and set errno to EAGAIN, or EWOULDBLOCK.

Or are you expecting it to return zero?

No, return -1 if no bytes were read and EAGAIN was the reason.

And in either case, how would you expect the application to detemine that either the error was due to no data or that the zero return is not EOF, and so to continue calling gzread()?

By detecting the -1 return and checking errno, i.e. as for read(2).

Through some other call?

I think I though about that, but IMNSHO it would be simpler, if possible, for gzread to become a drop-in replacement for read(2).

If it is not possible i.e. if this updated version of gzread hit some inconsistency or broke something else, then I think the only option would be to duplicate gzread.c with a gzreadnonblocking.c, with unique call names and special code to handle non-blocking. That would be a bit of a mess, but it would protect any existing applications.

If gzread() returns some data (so there is no error or zero indication), but in the process it hit an EAGAIN, does the application want to know that?

No, as with read(2), it has the number of bytes returned before the EAGAIN; the application can find out about the EAGAIN on the next call.

How should the other functions work with non-blocking input, i.e. gzfread(), gzgetc(), gzgets()?

I don't use those, so at this point I have no opinion about those.

Should there be an option for gzopen() or gzdopen() request or set O_NONBLOCK?

Possibly, but at this point it is not necessary.

[madler]

Thank you for your quick and detailed reply!

but you are zlib's BDFL.

Had to look that one up, but I suppose so. At least I think the B is correct.

To summarize, it's ok if I discard information about choked reads, by continuing to call read() in a loop even if it returns a partial buffer, and as well by returning a non-zero amount of data that has been uncompressed when read() returns -1 with EAGAIN (or EWOULDBLOCK), with no indication of the choke.

In that latter case, should I clear errno to avoid confusion, or leave it to provide some indication?

i.e. as much as possible, exactly as for read(2)

Then I suppose there should also be no need to call gzclearerr(). You should be able to call gzread() again after an EAGAIN.

[drbitboy]

Then I suppose there should also be no need to call gzclearerr(). You should be able to call gzread() again after an EAGAIN.

If you mean that you would rewrite the zlib gzread logic to make calling gzclearerr() by the gzread()-caller unnecessary, then yes, that would be nice, but maybe not a good idea if it breaks a zillion other applications.

TBF, even read(2)-callers sometimes need to clear errno, IIRC.

[madler]

TBF, even read(2)-callers sometimes need to clear errno, IIRC.

Do you know why?

[drbitboy]

system routines write non-zero values into errno; it is up to the application to clear errno to 0.

Usually, the -1 (or other negative) return value is the indication for a failure, but sometimes that -1 just triggers alternate logic (e.g. a wait before the next non-blocking read) that is dependent on the value of errno, so it is simpler to clear errno to 0 and watch errno instead of the return value.

I can't think of a specific instance, but I am fairly certain I have done it.

I haven't looked at why, but IIRC clearing the error in the gz struct via gzclearerr() is necessary, because otherwise [the next] gzread short-circuits without trying to read anything.

eta: [the next]

[madler]

Thanks.

I haven't looked at why, but IIRC clearing the error in the gz struct via gzclearerr() is necessary, because otherwise gzread short-circuits without trying to read anything.

Yes. I will fix that for when the last error was no data on a non-blocking read.

[drbitboy]

Thanks.

I haven't looked at why, but IIRC clearing the error in the gz struct via gzclearerr() is necessary, because otherwise gzread short-circuits without trying to read anything.

Yes. I will fix that for when the last error was no data on a non-blocking read.

I would be worried about some downstream application relying on the current behavior.