On Mon, 9 Nov 2009, L S wrote:

> Curl_socket_ready do return the correct value. Since openSSL want us to
> write information to the socket after a SSL_ERROR_WANT_WRITE error code it
> seems reasonable that Curl_socket_ready should return 2 (CURL_CSELECT_OUT).
> But Curl_readwrite only calls the recieve function if we have a readable
> socket. And therein lies the fault.

Exactly.

We need to separate what to wait for and what to do when the waited-for action
occurs since in this case the socket gets writable and we should still call
the read function to take care of it.

When thinking about it, I think we also have the same error for SSH although
we may not yet have found a case where it becomes visible.

> Insted of using the return value from the Curl_socket_ready (which isn't a
> good measure of determine what to do since both SSL_write and SSL_read might
> use both readable and writeable sockets during a renegotiation ) we need
> some other way of knowing what we are doing. But I don't really see an easy
> way of doing this.

Curl_socket_ready() is a rather low-level function that needs to stay working
exactly like it does.

I suggest we introduce a new internal struct member or two that allows the
protocol-specific parts to set a forced action to assume, and the
Curl_readwrite() function would then check that field first before it assumes
that the event implies a specific action.

It could work in a way similar to this (throwing out a suggestion):

Index: lib/transfer.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/transfer.c,v
retrieving revision 1.442
diff -u -r1.442 transfer.c
--- lib/transfer.c 12 Nov 2009 14:36:34 -0000 1.442
+++ lib/transfer.c 15 Nov 2009 14:28:12 -0000
@@ -1649,6 +1649,7 @@
    curl_socket_t fd_read;
    curl_socket_t fd_write;
    int select_res = conn->cselect_bits;
+ int act_data; /* how to act on the data */

    conn->cselect_bits = 0;

@@ -1675,21 +1676,24 @@
      return CURLE_SEND_ERROR;
    }

- /* We go ahead and do a read if we have a readable socket or if
- the stream was rewound (in which case we have data in a
- buffer) */
+ /* We go ahead and do a read if we have a readable socket or if the stream
+ was rewound (in which case we have data in a buffer) */
    if((k->keepon & KEEP_RECV) &&
- ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound)) {
+ ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound))
+ act_data |= (conn->forcewrite?KEEP_SEND:KEEP_RECV);

+ /* If we still have writing to do, we check if we have a writable socket. */
+ if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT))
+ act_data |= (conn->forceread?KEEP_RECV:KEEP_SEND);
+
+ if(act_data & KEEP_RECV) {
      result = readwrite_data(data, conn, k, &didwhat, done);
      if(result || *done)
        return result;
    }

- /* If we still have writing to do, we check if we have a writable socket. */
- if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT)) {
+ if(act_data & KEEP_SEND) {
      /* write */
-
      result = readwrite_upload(data, conn, k, &didwhat);
      if(result)
        return result;

It shouldn't be too hard to adapt the SSL stuff to use this concept methinks.
What you others think?

> One way ( but a quite ugly way I would say ) of solving the problem is to
> solve the problem in the Curl_ossl_recv function by creating a loop until we
> don't have a SSL_ERROR_WANT_WRITE or SSL_ERROR_WANT_READ error code ( or
> until things times out ).

We're already working hard to decrease the amount of blocking places within
the code and I think adding new blocks without very good reasons is a bad
idea. I much rather prefer we go the route I suggest above and polish it to
work...

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html