Author: rhuijben Date: Mon Nov 16 23:35:44 2015 New Revision: 1714710 URL: http://svn.apache.org/viewvc?rev=1714710&view=rev Log: Tweak the connection closed handling on incoming connections to make it less likely that connection errors accidentally close the server.
* incoming.c (read_from_client): Remove closed connections from pollset. (serf__process_client): Copy some code from connection. (serf_incoming_create2): Init new var. * serf.h (serf_incoming_closed_t): Allow returning error. * serf_private.h (serf_incoming_t): Add value. Modified: serf/trunk/incoming.c serf/trunk/serf.h serf/trunk/serf_private.h Modified: serf/trunk/incoming.c URL: http://svn.apache.org/viewvc/serf/trunk/incoming.c?rev=1714710&r1=1714709&r2=1714710&view=diff ============================================================================== --- serf/trunk/incoming.c (original) +++ serf/trunk/incoming.c Mon Nov 16 23:35:44 2015 @@ -279,7 +279,7 @@ serf_incoming_request_t *serf__incoming_ static apr_status_t read_from_client(serf_incoming_t *client) { - apr_status_t status; + apr_status_t status = APR_SUCCESS; serf_incoming_request_t *rq; if (client->proto_peek_bkt) @@ -293,7 +293,8 @@ static apr_status_t read_from_client(ser return client->perform_read(client); } - do { + while (status == APR_SUCCESS) { + rq = client->current_request; if (!rq) { serf_bucket_t *read_bkt; @@ -341,15 +342,48 @@ static apr_status_t read_from_client(ser status = destroy_request(rq); } - /* ### TODO: Check if connection is also at EOF? */ - status = APR_SUCCESS; + /* Is the connection at eof or just the request? */ + { + const char *data; + apr_size_t len; + + status = serf_bucket_peek(client->stream, &data, &len); + } } } - while (status == APR_SUCCESS); - if (APR_STATUS_IS_EAGAIN(status) || status == SERF_ERROR_WAIT_CONN) + if (!SERF_BUCKET_READ_ERROR(status) && !APR_STATUS_IS_EOF(status)) return APR_SUCCESS; + { + apr_pollfd_t tdesc = { 0 }; + int i; + + /* Remove us from the pollset */ + tdesc.desc_type = APR_POLL_SOCKET; + tdesc.desc.s = client->skt; + tdesc.reqevents = client->reqevents; + client->ctx->pollset_rm(client->ctx->pollset_baton, + &tdesc, &client->baton); + + /* And from the incommings list */ + for (i = 0; i < client->ctx->incomings->nelts; i++) { + if (GET_INCOMING(client->ctx, i) == client) { + GET_INCOMING(client->ctx, i) + = GET_INCOMING(client->ctx, + client->ctx->incomings->nelts - 1); + break; + } + } + client->ctx->incomings->nelts--; + client->seen_in_pollset |= APR_POLLHUP; /* No more events */ + } + + status = client->closed(client, client->closed_baton, status, + client->pool); + + /* ### Somehow do a apr_pool_destroy(client->pool); */ + return status; } @@ -570,6 +604,13 @@ apr_status_t serf__process_client(serf_i if (status) { return status; } + + /* If we decided to close our connection, return now as we don't + * want to write. + */ + if ((client->seen_in_pollset & APR_POLLHUP) != 0) { + return APR_SUCCESS; + } } if ((events & APR_POLLHUP) != 0) { @@ -716,6 +757,7 @@ apr_status_t serf_incoming_create2( ic->desc.desc_type = APR_POLL_SOCKET; ic->desc.desc.s = ic->skt; ic->desc.reqevents = APR_POLLIN | APR_POLLERR | APR_POLLHUP; + ic->seen_in_pollset = 0; /* Store the connection specific info in the configuration store */ /* ### Doesn't work... Doesn't support listeners yet*/ Modified: serf/trunk/serf.h URL: http://svn.apache.org/viewvc/serf/trunk/serf.h?rev=1714710&r1=1714709&r2=1714710&view=diff ============================================================================== --- serf/trunk/serf.h (original) +++ serf/trunk/serf.h Mon Nov 16 23:35:44 2015 @@ -379,7 +379,7 @@ typedef void (*serf_connection_closed_t) /** * Like serf_connection_closed_t, but applies to incoming connections. */ -typedef void(*serf_incoming_closed_t)( +typedef apr_status_t (*serf_incoming_closed_t)( serf_incoming_t *incoming, void *closed_baton, apr_status_t why, Modified: serf/trunk/serf_private.h URL: http://svn.apache.org/viewvc/serf/trunk/serf_private.h?rev=1714710&r1=1714709&r2=1714710&view=diff ============================================================================== --- serf/trunk/serf_private.h (original) +++ serf/trunk/serf_private.h Mon Nov 16 23:35:44 2015 @@ -352,18 +352,20 @@ struct serf_listener_t { struct serf_incoming_t { serf_context_t *ctx; + serf_io_baton_t baton; serf_incoming_request_setup_t req_setup; void *req_setup_baton; apr_socket_t *skt; /* Lives in parent of POOL */ - apr_pool_t *pool; + apr_pool_t *pool; serf_bucket_alloc_t *allocator; apr_pollfd_t desc; /* the last reqevents we gave to pollset_add */ apr_int16_t reqevents; + apr_int16_t seen_in_pollset; struct iovec vec[IOV_MAX]; int vec_len; @@ -510,7 +512,7 @@ struct serf_connection_t { /* Host url, path ommitted, syntax: https://svn.apache.org . */ const char *host_url; - + /* Exploded host url, path ommitted. Only scheme, hostinfo, hostname & port values are filled in. */ apr_uri_t host_info; @@ -566,10 +568,10 @@ void serf__bucket_drain(serf_bucket_t *b /** Transform a response_bucket in-place into an aggregate bucket. Restore the status line and all headers, not just the body. - + This can only be used when we haven't started reading the body of the response yet. - + Keep internal for now, probably only useful within serf. */ apr_status_t serf_response_full_become_aggregate(serf_bucket_t *bucket); @@ -629,7 +631,7 @@ apr_status_t serf__handle_auth_response( /* Get the cached serf__authn_info_t object for the target server, or create one when this is the first connection to the server. TODO: The serf__authn_info_t objects are allocated in the context pool, so - a context that's used to connect to many different servers using Basic or + a context that's used to connect to many different servers using Basic or Digest authencation will hold on to many objects indefinitely. We should be able to cleanup stale objects from time to time. */ serf__authn_info_t *serf__get_authn_info_for_server(serf_connection_t *conn); @@ -722,7 +724,7 @@ apr_status_t serf__handle_response(serf_ /** Logging functions. **/ -/* Initialize the logging subsystem. This will store a log baton in the +/* Initialize the logging subsystem. This will store a log baton in the context's configuration store. */ apr_status_t serf__log_init(serf_context_t *ctx);