Handle idle stream in priority field

lib/nghttp2_session.c

@@ -588,7 +588,28 @@ int nghttp2_session_reprioritize_stream
   if(pri_spec->stream_id != 0) {
     dep_stream = nghttp2_session_get_stream_raw(session, pri_spec->stream_id);
 
-    if(!dep_stream || !nghttp2_stream_in_dep_tree(dep_stream)) {
+    if(session->server && !dep_stream &&
+       session_detect_idle_stream(session, pri_spec->stream_id)) {
+      nghttp2_session_adjust_closed_stream(session, 1);
+
+      nghttp2_priority_spec_default_init(&pri_spec_default);
+
+      if(nghttp2_session_can_add_closed_stream(session, 1)) {
+
+        dep_stream = nghttp2_session_open_stream(session,
+                                                 pri_spec->stream_id,
+                                                 NGHTTP2_FLAG_NONE,
+                                                 &pri_spec_default,
+                                                 NGHTTP2_STREAM_IDLE,
+                                                 NULL);
+
+        if(dep_stream == NULL) {
+          return NGHTTP2_ERR_NOMEM;
+        }
+      } else {
+        pri_spec = &pri_spec_default;
+      }
+    } else if(!dep_stream || !nghttp2_stream_in_dep_tree(dep_stream)) {
       nghttp2_priority_spec_default_init(&pri_spec_default);
       pri_spec = &pri_spec_default;
     }
@@ -822,6 +843,7 @@ nghttp2_stream* nghttp2_session_open_stream(nghttp2_session *session,
   int stream_alloc = 0;
   nghttp2_priority_spec pri_spec_default;
   nghttp2_priority_spec *pri_spec = pri_spec_in;
+  ssize_t num_adjust_closed = 0;
 
   stream = nghttp2_session_get_stream_raw(session, stream_id);
 
@@ -834,6 +856,7 @@ nghttp2_stream* nghttp2_session_open_stream(nghttp2_session *session,
     if(session->server &&
        (!nghttp2_session_is_my_stream_id(session, stream_id) ||
         initial_state == NGHTTP2_STREAM_IDLE)) {
+      num_adjust_closed = 1;
       nghttp2_session_adjust_closed_stream(session, 1);
     }
 
@@ -848,9 +871,35 @@ 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 dep_stream is not part of dependency tree, stream will get
-       default priority. */
-    if(!dep_stream || !nghttp2_stream_in_dep_tree(dep_stream)) {
+    if(session->server && !dep_stream &&
+       session_detect_idle_stream(session, pri_spec->stream_id)) {
+      ++num_adjust_closed;
+      nghttp2_session_adjust_closed_stream(session, num_adjust_closed);
+
+      nghttp2_priority_spec_default_init(&pri_spec_default);
+
+      if(nghttp2_session_can_add_closed_stream(session, num_adjust_closed)) {
+
+        dep_stream = nghttp2_session_open_stream(session,
+                                                 pri_spec->stream_id,
+                                                 NGHTTP2_FLAG_NONE,
+                                                 &pri_spec_default,
+                                                 NGHTTP2_STREAM_IDLE,
+                                                 NULL);
+
+        if(dep_stream == NULL) {
+          if(stream_alloc) {
+            free(stream);
+          }
+
+          return NULL;
+        }
+      } else {
+        pri_spec = &pri_spec_default;
+      }
+    } else if(!dep_stream || !nghttp2_stream_in_dep_tree(dep_stream)) {
+      /* If dep_stream is not part of dependency tree, stream will get
+         default priority. */
       nghttp2_priority_spec_default_init(&pri_spec_default);
       pri_spec = &pri_spec_default;
     }
@@ -1082,17 +1131,15 @@ void nghttp2_session_detach_closed_stream(nghttp2_session *session,
   --session->num_closed_streams;
 }
 
-/* Returns nonzero if closed stream can not be added to linked list
-   now. */
-static int session_closed_stream_full(nghttp2_session *session)
+int nghttp2_session_can_add_closed_stream(nghttp2_session *session,
+                                          ssize_t offset)
 {
   size_t num_stream_max;
 
   num_stream_max = nghttp2_min(session->local_settings.max_concurrent_streams,
                                session->pending_local_max_concurrent_stream);
 
-  return (size_t)nghttp2_max(0, (ssize_t)session->num_closed_streams - 1) +
-    session->num_incoming_streams >= num_stream_max;
+  return offset + session->num_incoming_streams <= num_stream_max;
 }
 
 void nghttp2_session_adjust_closed_stream(nghttp2_session *session,
@@ -3304,7 +3351,7 @@ int nghttp2_session_on_priority_received(nghttp2_session *session,
     /* PRIORITY against idle stream can create anchor node in
        dependency tree. */
     if(!session_detect_idle_stream(session, frame->hd.stream_id) ||
-       session_closed_stream_full(session)) {
+       !nghttp2_session_can_add_closed_stream(session, 1)) {
       return 0;
     }
 

lib/nghttp2_session.h

@@ -431,6 +431,13 @@ void nghttp2_session_keep_closed_stream(nghttp2_session *session,
 void nghttp2_session_detach_closed_stream(nghttp2_session *session,
                                           nghttp2_stream *stream);
 
+/*
+ * Returns nonzero if |offset| closed stream(s) can be added to closed
+ * linked list now.
+ */
+int nghttp2_session_can_add_closed_stream(nghttp2_session *session,
+                                          ssize_t offset);
+
 /*
  * Deletes closed stream to ensure that number of incoming streams
  * including active and closed is in the maximum number of allowed

tests/main.c

@@ -149,6 +149,8 @@ int main(int argc, char* argv[])
       !CU_add_test(pSuite, "session_upgrade", test_nghttp2_session_upgrade) ||
       !CU_add_test(pSuite, "session_reprioritize_stream",
                    test_nghttp2_session_reprioritize_stream) ||
+      !CU_add_test(pSuite, "session_reprioritize_stream_with_closed_stream_limit",
+                   test_nghttp2_session_reprioritize_stream_with_closed_stream_limit) ||
       !CU_add_test(pSuite, "submit_data", test_nghttp2_submit_data) ||
       !CU_add_test(pSuite, "submit_data_read_length_too_large",
                    test_nghttp2_submit_data_read_length_too_large) ||
@@ -187,6 +189,8 @@ int main(int argc, char* argv[])
                    test_nghttp2_submit_invalid_nv) ||
       !CU_add_test(pSuite, "session_open_stream",
                    test_nghttp2_session_open_stream) ||
+      !CU_add_test(pSuite, "session_open_stream_with_closed_stream_limit",
+                   test_nghttp2_session_open_stream_with_closed_stream_limit) ||
       !CU_add_test(pSuite, "session_get_next_ob_item",
                    test_nghttp2_session_get_next_ob_item) ||
       !CU_add_test(pSuite, "session_pop_next_ob_item",

tests/nghttp2_session_test.c

@@ -3049,20 +3049,27 @@ void test_nghttp2_session_reprioritize_stream(void)
   CU_ASSERT(10 == stream->weight);
   CU_ASSERT(NULL == stream->dep_prev);
 
-  /* If dep_stream does not exist, default priority is assigned. */
+  /* If depenency to idle stream which is not in depdenency tree yet */
 
   nghttp2_priority_spec_init(&pri_spec, 3, 99, 0);
 
   nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
-  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
-  CU_ASSERT(NULL == stream->dep_prev);
+  CU_ASSERT(99 == stream->weight);
+  CU_ASSERT(3 == stream->dep_prev->stream_id);
+
+  dep_stream = nghttp2_session_get_stream_raw(session, 3);
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == dep_stream->weight);
 
   dep_stream = open_stream(session, 3);
 
+  /* Change weight */
+  pri_spec.weight = 128;
+
   nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
 
-  CU_ASSERT(99 == stream->weight);
+  CU_ASSERT(128 == stream->weight);
   CU_ASSERT(dep_stream == stream->dep_prev);
 
   /* Test circular dependency; stream 1 is first removed and becomes
@@ -3074,6 +3081,59 @@ void test_nghttp2_session_reprioritize_stream(void)
   CU_ASSERT(1 == dep_stream->weight);
   CU_ASSERT(stream == dep_stream->dep_prev);
 
+  /* Making priority to closed stream will result in default
+     priority */
+  session->last_recv_stream_id = 9;
+
+  nghttp2_priority_spec_init(&pri_spec, 5, 5, 0);
+
+  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
+  nghttp2_session_del(session);
+}
+
+void test_nghttp2_session_reprioritize_stream_with_closed_stream_limit(void)
+{
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_stream *stream;
+  nghttp2_priority_spec pri_spec;
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.send_callback = block_count_send_callback;
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  stream = nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE,
+                                       &pri_spec_default,
+                                       NGHTTP2_STREAM_OPENING, NULL);
+
+  session->pending_local_max_concurrent_stream = 1;
+
+  nghttp2_priority_spec_init(&pri_spec, 101, 10, 0);
+
+  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+
+  /* No room to create idle stream, so default priority was applied. */
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+  CU_ASSERT(NULL == stream->dep_prev);
+
+  session->pending_local_max_concurrent_stream = 2;
+
+  /* Now idle stream can be created */
+
+  nghttp2_session_reprioritize_stream(session, stream, &pri_spec);
+
+  CU_ASSERT(10 == stream->weight);
+  CU_ASSERT(101 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream_raw(session, 101);
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
   nghttp2_session_del(session);
 }
 
@@ -4233,15 +4293,31 @@ void test_nghttp2_session_open_stream(void)
   CU_ASSERT(17 == stream->weight);
   CU_ASSERT(1 == stream->dep_prev->stream_id);
 
-  /* Dependency to non-existent stream will become default priority */
+  /* Dependency to idle stream */
   nghttp2_priority_spec_init(&pri_spec, 1000000007, 240, 1);
 
   stream = nghttp2_session_open_stream(session, 5, NGHTTP2_STREAM_FLAG_NONE,
                                        &pri_spec, NGHTTP2_STREAM_OPENED,
                                        NULL);
+  CU_ASSERT(240 == stream->weight);
+  CU_ASSERT(1000000007 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream_raw(session, 1000000007);
+
   CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
   CU_ASSERT(NULL != stream->root_next);
 
+  /* Dependency to closed stream which is not in dependency tree */
+  session->last_recv_stream_id = 7;
+
+  nghttp2_priority_spec_init(&pri_spec, 7, 10, 0);
+
+  stream = nghttp2_session_open_stream(session, 9, NGHTTP2_FLAG_NONE,
+                                       &pri_spec, NGHTTP2_STREAM_OPENED,
+                                       NULL);
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
   nghttp2_session_del(session);
 
   nghttp2_session_client_new(&session, &callbacks, NULL);
@@ -4258,6 +4334,65 @@ void test_nghttp2_session_open_stream(void)
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_session_open_stream_with_closed_stream_limit(void)
+{
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_stream *stream;
+  nghttp2_priority_spec pri_spec;
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  session->pending_local_max_concurrent_stream = 1;
+
+  /* Dependency to idle stream */
+  nghttp2_priority_spec_init(&pri_spec, 101, 245, 0);
+
+  stream = nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE,
+                                       &pri_spec, NGHTTP2_STREAM_OPENED,
+                                       NULL);
+
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
+  session->pending_local_max_concurrent_stream = 3;
+
+  /* Now another 2 streams can be added */
+  stream = nghttp2_session_open_stream(session, 3, NGHTTP2_STREAM_FLAG_NONE,
+                                       &pri_spec, NGHTTP2_STREAM_OPENED,
+                                       NULL);
+
+  CU_ASSERT(245 == stream->weight);
+  CU_ASSERT(101 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream_raw(session, 101);
+
+  CU_ASSERT(NGHTTP2_STREAM_IDLE == stream->state);
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
+  session->pending_local_max_concurrent_stream = 4;
+
+  /* Now another 1 stream can be added */
+
+  nghttp2_priority_spec_init(&pri_spec, 211, 1, 0);
+
+  /* stream 101 was already created and does not consume another
+     limit. */
+  stream = nghttp2_session_open_stream(session, 101, NGHTTP2_STREAM_FLAG_NONE,
+                                       &pri_spec, NGHTTP2_STREAM_OPENED,
+                                       NULL);
+
+  CU_ASSERT(1 == stream->weight);
+  CU_ASSERT(211 == stream->dep_prev->stream_id);
+
+  stream = nghttp2_session_get_stream_raw(session, 211);
+
+  CU_ASSERT(NGHTTP2_STREAM_IDLE == stream->state);
+  CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight);
+
+  nghttp2_session_del(session);
+}
+
 void test_nghttp2_session_get_next_ob_item(void)
 {
   nghttp2_session *session;

tests/nghttp2_session_test.h

@@ -61,6 +61,7 @@ void test_nghttp2_session_send_push_promise(void);
 void test_nghttp2_session_is_my_stream_id(void);
 void test_nghttp2_session_upgrade(void);
 void test_nghttp2_session_reprioritize_stream(void);
+void test_nghttp2_session_reprioritize_stream_with_closed_stream_limit(void);
 void test_nghttp2_submit_data(void);
 void test_nghttp2_submit_data_read_length_too_large(void);
 void test_nghttp2_submit_data_read_length_smallest(void);
@@ -83,6 +84,7 @@ void test_nghttp2_submit_window_update_local_window_size(void);
 void test_nghttp2_submit_altsvc(void);
 void test_nghttp2_submit_invalid_nv(void);
 void test_nghttp2_session_open_stream(void);
+void test_nghttp2_session_open_stream_with_closed_stream_limit(void);
 void test_nghttp2_session_get_next_ob_item(void);
 void test_nghttp2_session_pop_next_ob_item(void);
 void test_nghttp2_session_reply_fail(void);