Hi everyone,

as I already posted on this list before, I've created a fairly large
patch for better IMAP support in libcurl [1]. I'm in contact with
Steve Holme as the maintainer of the IMAP parts of the library and
he'll be doing the reviewing, splitting and splicing of most of the
changes. But he recommended me to ask for a wider agreement regarding
modifications in the shared pingpong code and that's what this message
is about.

I made two pingpong commits, both dealing with the same problem - an
unnecessary call to Curl_socket_ready(), resulting in delays or a
hang. I'll present an IMAP example.

1/ A big chunk of data is received, containing the first untagged FETCH
    response, the message data itself and even the final FETCH tagged
    response.
2/ imap_state_fetch_resp() processes the first line and extracts the
    whole message data as well, as it is already available in the PP
    cache.
3/ The handle goes through PERFORM state into DONE without doing
    anything - the download has already finished.
4/ In imap_done(), the final line of the response needs to be read to be
    sure the command finished OK and to clear the connection before
    further use (e.g. SocketIsDead() would mark it unusable otherwise).
    But although the data are already in the PP cache, they will not be
    processed, because Curl_pp_statemach() will hang in
    Curl_socket_ready() (blocking version) or it will just return straight
    back, because there is seemingly nothing to receive.

In order to avoid that, I created a new function Curl_pp_moredata(),
which returns whether the PP machinery is in the above-mentioned state
[2], and then changed Curl_pp_statemach() to not check the socket for
more data when it still has some cached and just invoke the processing
callback [3].

The new function is a global one (and even a separate function, for
that matter) because at least IMAP might need to detect this exact
condition in its imap_statemach_act() too: To avoid the
imap_matchresp() callback growing too long, I proposed handling even
the untagged responses in the per-state response handler, like
imap_state_capability_resp() [4]. Curl_pp_readresp() then needs to be
called repeatedly till everything in the cache has been processed [5],
otherwise a similar lockup or delays can occur.

I'll appreciate any comments on whether this is an acceptable
solution, if it has any negative effects in other protocols I
overlooked or just how to do it all better.

Thank you,
Jiri

[1] https://github.com/yirkha/curl/compare/master...imap
[2] https://github.com/yirkha/curl/commit/ce257400fffba51cbf9f08fa3cc24c3e033bd5f5
[3] https://github.com/yirkha/curl/commit/fcc2e08def2d3761940503d64714236ae4384b86
[4] https://github.com/yirkha/curl/commit/2f421c01fb4e44f4ab8da24a5b0390e149561c8f
[5] https://github.com/yirkha/curl/commit/ae53f4799f5cb2f967178cfc9058d5a5967fc081
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-02-20