From 894c1bd02ed71be5f684653d27749de541226f6f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 24 Dec 2015 20:53:08 +0900 Subject: [PATCH] Create idle stream on client side With the presence of idle stream related API (e.g., nghttp2_create_idle_stream()), it is more predictable for client to create idle streams with its dependency to another idle stream. Previously, we didn't create complete parent idle stream in this case. Now we create idle streams as we do on server side. --- lib/nghttp2_session.c | 27 +++++---- tests/main.c | 2 + tests/nghttp2_session_test.c | 106 ++++++++++++++++++++++++++++++++++- tests/nghttp2_session_test.h | 1 + 4 files changed, 125 insertions(+), 11 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index f94a009a..e4dd4f97 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -630,7 +630,7 @@ int nghttp2_session_reprioritize_stream( if (pri_spec->stream_id != 0) { dep_stream = nghttp2_session_get_stream_raw(session, pri_spec->stream_id); - if (session->server && !dep_stream && + if (!dep_stream && session_detect_idle_stream(session, pri_spec->stream_id)) { nghttp2_priority_spec_default_init(&pri_spec_default); @@ -899,7 +899,7 @@ nghttp2_stream *nghttp2_session_open_stream(nghttp2_session *session, if (pri_spec->stream_id != 0) { dep_stream = nghttp2_session_get_stream_raw(session, pri_spec->stream_id); - if (session->server && !dep_stream && + if (!dep_stream && session_detect_idle_stream(session, pri_spec->stream_id)) { /* Depends on idle stream, which does not exist in memory. Assign default priority for it. */ @@ -964,7 +964,6 @@ nghttp2_stream *nghttp2_session_open_stream(nghttp2_session *session, case NGHTTP2_STREAM_IDLE: /* Idle stream does not count toward the concurrent streams limit. This is used as anchor node in dependency tree. */ - assert(session->server); nghttp2_session_keep_idle_stream(session, stream); break; default: @@ -2357,14 +2356,22 @@ static int session_after_frame_sent1(nghttp2_session *session) { stream = nghttp2_session_get_stream_raw(session, frame->hd.stream_id); if (!stream) { - break; - } + if (!session_detect_idle_stream(session, frame->hd.stream_id)) { + break; + } - rv = nghttp2_session_reprioritize_stream(session, stream, - &frame->priority.pri_spec); - - if (nghttp2_is_fatal(rv)) { - return rv; + stream = nghttp2_session_open_stream( + session, frame->hd.stream_id, NGHTTP2_FLAG_NONE, + &frame->priority.pri_spec, NGHTTP2_STREAM_IDLE, NULL); + if (!stream) { + return NGHTTP2_ERR_NOMEM; + } + } else { + rv = nghttp2_session_reprioritize_stream(session, stream, + &frame->priority.pri_spec); + if (nghttp2_is_fatal(rv)) { + return rv; + } } rv = nghttp2_session_adjust_idle_stream(session); diff --git a/tests/main.c b/tests/main.c index 4bb7f041..5d1c2533 100644 --- a/tests/main.c +++ b/tests/main.c @@ -292,6 +292,8 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_session_change_stream_priority) || !CU_add_test(pSuite, "session_repeated_priority_change", test_nghttp2_session_repeated_priority_change) || + !CU_add_test(pSuite, "session_repeated_priority_submission", + test_nghttp2_session_repeated_priority_submission) || !CU_add_test(pSuite, "http_mandatory_headers", test_nghttp2_http_mandatory_headers) || !CU_add_test(pSuite, "http_content_length", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index b18c60ad..6fb2768e 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -8535,7 +8535,7 @@ void test_nghttp2_session_flooding(void) { void test_nghttp2_session_change_stream_priority(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; - nghttp2_stream *stream1, *stream2, *stream3; + nghttp2_stream *stream1, *stream2, *stream3, *stream5; nghttp2_priority_spec pri_spec; int rv; @@ -8568,6 +8568,41 @@ void test_nghttp2_session_change_stream_priority(void) { rv = nghttp2_session_change_stream_priority(session, 0, &pri_spec); CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + /* Depends on the non-existing idle stream. This creates that idle + stream. */ + nghttp2_priority_spec_init(&pri_spec, 5, 9, 1); + + rv = nghttp2_session_change_stream_priority(session, 2, &pri_spec); + + CU_ASSERT(0 == rv); + + stream5 = nghttp2_session_get_stream_raw(session, 5); + + CU_ASSERT(NULL != stream5); + CU_ASSERT(&session->root == stream5->dep_prev); + CU_ASSERT(stream5 == stream2->dep_prev); + CU_ASSERT(9 == stream2->weight); + + nghttp2_session_del(session); + + /* Check that this works in client session too */ + nghttp2_session_client_new(&session, &callbacks, NULL); + + stream1 = open_stream(session, 1); + + nghttp2_priority_spec_init(&pri_spec, 5, 9, 1); + + rv = nghttp2_session_change_stream_priority(session, 1, &pri_spec); + + CU_ASSERT(0 == rv); + + stream5 = nghttp2_session_get_stream_raw(session, 5); + + CU_ASSERT(NULL != stream5); + CU_ASSERT(&session->root == stream5->dep_prev); + CU_ASSERT(stream5 == stream1->dep_prev); + CU_ASSERT(9 == stream1->weight); + nghttp2_session_del(session); } @@ -8640,6 +8675,27 @@ void test_nghttp2_session_create_idle_stream(void) { CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); nghttp2_session_del(session); + + /* Check that this works in client session too */ + nghttp2_session_client_new(&session, &callbacks, NULL); + + nghttp2_priority_spec_init(&pri_spec, 4, 99, 1); + + rv = nghttp2_session_create_idle_stream(session, 2, &pri_spec); + + CU_ASSERT(0 == rv); + + stream4 = nghttp2_session_get_stream_raw(session, 4); + stream2 = nghttp2_session_get_stream_raw(session, 2); + + CU_ASSERT(NULL != stream4); + CU_ASSERT(NULL != stream2); + CU_ASSERT(&session->root == stream4->dep_prev); + CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream4->weight); + CU_ASSERT(stream4 == stream2->dep_prev); + CU_ASSERT(99 == stream2->weight); + + nghttp2_session_del(session); } void test_nghttp2_session_repeated_priority_change(void) { @@ -8693,6 +8749,54 @@ void test_nghttp2_session_repeated_priority_change(void) { nghttp2_session_del(session); } +void test_nghttp2_session_repeated_priority_submission(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_priority_spec pri_spec; + int32_t stream_id, last_stream_id; + uint32_t max_streams = NGHTTP2_MIN_IDLE_STREAMS; + + memset(&callbacks, 0, sizeof(callbacks)); + + callbacks.send_callback = null_send_callback; + + nghttp2_session_client_new(&session, &callbacks, NULL); + + session->local_settings.max_concurrent_streams = max_streams; + + /* 1 -> 0 */ + nghttp2_priority_spec_init(&pri_spec, 0, 16, 0); + + CU_ASSERT(0 == + nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, 1, &pri_spec)); + + last_stream_id = (int32_t)(max_streams * 2 + 1); + + for (stream_id = 3; stream_id < last_stream_id; stream_id += 2) { + /* 1 -> stream_id */ + nghttp2_priority_spec_init(&pri_spec, stream_id, 16, 0); + + CU_ASSERT( + 0 == nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, 1, &pri_spec)); + } + + CU_ASSERT(0 == nghttp2_session_send(session)); + CU_ASSERT(max_streams == session->num_idle_streams); + CU_ASSERT(1 == session->idle_stream_head->stream_id); + + /* 1 -> last_stream_id */ + nghttp2_priority_spec_init(&pri_spec, last_stream_id, 16, 0); + + CU_ASSERT(0 == + nghttp2_submit_priority(session, NGHTTP2_FLAG_NONE, 1, &pri_spec)); + + CU_ASSERT(0 == nghttp2_session_send(session)); + CU_ASSERT(max_streams == session->num_idle_streams); + CU_ASSERT(3 == session->idle_stream_head->stream_id); + + nghttp2_session_del(session); +} + static void check_nghttp2_http_recv_headers_fail( nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id, int stream_state, const nghttp2_nv *nva, size_t nvlen) { diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index cb23f4d4..3a54ba00 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -139,6 +139,7 @@ void test_nghttp2_session_flooding(void); void test_nghttp2_session_change_stream_priority(void); void test_nghttp2_session_create_idle_stream(void); void test_nghttp2_session_repeated_priority_change(void); +void test_nghttp2_session_repeated_priority_submission(void); void test_nghttp2_http_mandatory_headers(void); void test_nghttp2_http_content_length(void); void test_nghttp2_http_content_length_mismatch(void);