Clear 2 types of stream deferred flag indenpendently

Previously when nghttp2_stream_resume_deferred_data() is called,
deferred flags in stream->flags are all cleared.  This is not ideal
because if application returned NGHTTP2_ERR_DEFERRED, and also that
stream is deferred by flow control, then all flags are cleared and
read callback will be invoked again.  This commit fixes this issue.
This changes error condition of nghttp2_session_resume_data().
Previously we return error if stream was deferred by flow control.
Now we don't return error in this case.  We just clear
NGHTTP2_FLAG_DEFERRED_USER and if still
NGHTTP2_FLAG_DEFERRED_FLOW_CONTROL is set, just return 0.
This commit is contained in:
Tatsuhiro Tsujikawa 2014-09-26 21:32:17 +09:00
parent 937bb9f768
commit be0f6dcaaf
4 changed files with 28 additions and 23 deletions

View File

@ -2062,8 +2062,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session,
* negative error codes: * negative error codes:
* *
* :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
* The stream does not exist; or no deferred data exist; or data * The stream does not exist; or no deferred data exist.
* was deferred by flow control.
* :enum:`NGHTTP2_ERR_NOMEM` * :enum:`NGHTTP2_ERR_NOMEM`
* Out of memory. * Out of memory.
*/ */

View File

@ -3275,12 +3275,12 @@ static int update_remote_initial_window_size_func
/* If window size gets positive, push deferred DATA frame to /* If window size gets positive, push deferred DATA frame to
outbound queue. */ outbound queue. */
if(nghttp2_stream_check_deferred_by_flow_control(stream) && if(stream->remote_window_size > 0 &&
stream->remote_window_size > 0 && nghttp2_stream_check_deferred_by_flow_control(stream)) {
arg->session->remote_window_size > 0) {
rv = nghttp2_stream_resume_deferred_data(stream, &arg->session->ob_da_pq, rv = nghttp2_stream_resume_deferred_data
arg->session->last_cycle); (stream, NGHTTP2_STREAM_FLAG_DEFERRED_FLOW_CONTROL,
&arg->session->ob_da_pq, arg->session->last_cycle);
if(nghttp2_is_fatal(rv)) { if(nghttp2_is_fatal(rv)) {
return rv; return rv;
@ -3899,7 +3899,8 @@ static int session_on_stream_window_update_received
if(stream->remote_window_size > 0 && if(stream->remote_window_size > 0 &&
nghttp2_stream_check_deferred_by_flow_control(stream)) { nghttp2_stream_check_deferred_by_flow_control(stream)) {
rv = nghttp2_stream_resume_deferred_data(stream, &session->ob_da_pq, rv = nghttp2_stream_resume_deferred_data
(stream, NGHTTP2_STREAM_FLAG_DEFERRED_FLOW_CONTROL, &session->ob_da_pq,
session->last_cycle); session->last_cycle);
if(nghttp2_is_fatal(rv)) { if(nghttp2_is_fatal(rv)) {
@ -5863,13 +5864,12 @@ int nghttp2_session_resume_data(nghttp2_session *session, int32_t stream_id)
int rv; int rv;
nghttp2_stream *stream; nghttp2_stream *stream;
stream = nghttp2_session_get_stream(session, stream_id); stream = nghttp2_session_get_stream(session, stream_id);
if(stream == NULL || if(stream == NULL || !nghttp2_stream_check_deferred_data(stream)) {
nghttp2_stream_check_deferred_by_flow_control(stream) ||
!nghttp2_stream_check_deferred_data(stream)) {
return NGHTTP2_ERR_INVALID_ARGUMENT; return NGHTTP2_ERR_INVALID_ARGUMENT;
} }
rv = nghttp2_stream_resume_deferred_data(stream, &session->ob_da_pq, rv = nghttp2_stream_resume_deferred_data
(stream, NGHTTP2_STREAM_FLAG_DEFERRED_USER, &session->ob_da_pq,
session->last_cycle); session->last_cycle);
if(nghttp2_is_fatal(rv)) { if(nghttp2_is_fatal(rv)) {

View File

@ -383,15 +383,19 @@ int nghttp2_stream_defer_data(nghttp2_stream *stream, uint8_t flags,
return stream_update_dep_on_detach_data(stream, pq, cycle); return stream_update_dep_on_detach_data(stream, pq, cycle);
} }
int nghttp2_stream_resume_deferred_data(nghttp2_stream *stream, int nghttp2_stream_resume_deferred_data(nghttp2_stream *stream, uint8_t flags,
nghttp2_pq *pq, uint64_t cycle) nghttp2_pq *pq, uint64_t cycle)
{ {
assert(stream->data_item); assert(stream->data_item);
DEBUGF(fprintf(stderr, "stream: stream=%d resume data=%p\n", DEBUGF(fprintf(stderr, "stream: stream=%d resume data=%p flags=%02x\n",
stream->stream_id, stream->data_item)); stream->stream_id, stream->data_item, flags));
stream->flags &= ~NGHTTP2_STREAM_FLAG_DEFERRED_ALL; stream->flags &= ~flags;
if(stream->flags & NGHTTP2_STREAM_FLAG_DEFERRED_ALL) {
return 0;
}
return stream_update_dep_on_attach_data(stream, pq, cycle); return stream_update_dep_on_attach_data(stream, pq, cycle);
} }

View File

@ -216,12 +216,14 @@ int nghttp2_stream_defer_data(nghttp2_stream *stream, uint8_t flags,
nghttp2_pq *pq, uint64_t cycle); nghttp2_pq *pq, uint64_t cycle);
/* /*
* Detaches deferred data in this stream and it is back to active * Put back deferred data in this stream to active state. The |flags|
* state. The flags NGHTTP2_STREAM_FLAG_DEFERRED_USER and * are one or more of bitwise OR of the following values:
* NGHTTP2_STREAM_FLAG_DEFERRED_FLOW_CONTROL are cleared if they are * NGHTTP2_STREAM_FLAG_DEFERRED_USER and
* set. * NGHTTP2_STREAM_FLAG_DEFERRED_FLOW_CONTROL and given masks are
* cleared if they are set. So even if this function is called, if
* one of flag is still set, data does not become active.
*/ */
int nghttp2_stream_resume_deferred_data(nghttp2_stream *stream, int nghttp2_stream_resume_deferred_data(nghttp2_stream *stream, uint8_t flags,
nghttp2_pq *pq, uint64_t cycle); nghttp2_pq *pq, uint64_t cycle);
/* /*