Commit b2f88f8f authored by Tatsuhiro Tsujikawa's avatar Tatsuhiro Tsujikawa

Fix memory leak around stream->data_item

Previously we missed the case where stream->data_item is not deleted
and it caused leak.  Now stream->data_item is properly deleted when
session is deleted.  We decided not to delete data_item in
nghttp2_stream_free() since we need nghttp2_session to decide whether
data_item should be deleted or not there.
parent 44ac5710
...@@ -496,8 +496,21 @@ int nghttp2_session_server_new2(nghttp2_session **session_ptr, ...@@ -496,8 +496,21 @@ int nghttp2_session_server_new2(nghttp2_session **session_ptr,
static int free_streams(nghttp2_map_entry *entry, void *ptr) static int free_streams(nghttp2_map_entry *entry, void *ptr)
{ {
nghttp2_stream_free((nghttp2_stream*)entry); nghttp2_session *session;
free(entry); nghttp2_stream *stream;
nghttp2_outbound_item *item;
session = (nghttp2_session*)ptr;
stream = (nghttp2_stream*)entry;
item = stream->data_item;
if(item && !item->queued && item != session->aob.item) {
nghttp2_outbound_item_free(item);
free(item);
}
nghttp2_stream_free(stream);
free(stream);
return 0; return 0;
} }
...@@ -524,7 +537,7 @@ void nghttp2_session_del(nghttp2_session *session) ...@@ -524,7 +537,7 @@ void nghttp2_session_del(nghttp2_session *session)
/* Have to free streams first, so that we can check /* Have to free streams first, so that we can check
stream->data_item->queued */ stream->data_item->queued */
nghttp2_map_each_free(&session->streams, free_streams, NULL); nghttp2_map_each_free(&session->streams, free_streams, session);
nghttp2_map_free(&session->streams); nghttp2_map_free(&session->streams);
ob_pq_free(&session->ob_pq); ob_pq_free(&session->ob_pq);
...@@ -878,6 +891,7 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, ...@@ -878,6 +891,7 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id,
points to this item, let active_outbound_item_reset() points to this item, let active_outbound_item_reset()
free the item. */ free the item. */
if(!item->queued && item != session->aob.item) { if(!item->queued && item != session->aob.item) {
nghttp2_outbound_item_free(item);
free(item); free(item);
} }
} }
......
...@@ -73,12 +73,10 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, ...@@ -73,12 +73,10 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id,
void nghttp2_stream_free(nghttp2_stream *stream) void nghttp2_stream_free(nghttp2_stream *stream)
{ {
if(stream->flags & NGHTTP2_STREAM_FLAG_DEFERRED_ALL) { /* We don't free stream->data_item. If it is assigned to aob, then
nghttp2_outbound_item_free(stream->data_item); active_outbound_item_reset() will delete it. If it is queued,
free(stream->data_item); then it is deleted when pq is deleted in nghttp2_session_del().
} Otherwise, nghttp2_session_del() will delete it. */
/* We don't free stream->data_item otherwise. */
} }
void nghttp2_stream_shutdown(nghttp2_stream *stream, nghttp2_shut_flag flag) void nghttp2_stream_shutdown(nghttp2_stream *stream, nghttp2_shut_flag flag)
......
...@@ -245,6 +245,8 @@ int main(int argc, char* argv[]) ...@@ -245,6 +245,8 @@ int main(int argc, char* argv[])
test_nghttp2_session_on_header_temporal_failure) || test_nghttp2_session_on_header_temporal_failure) ||
!CU_add_test(pSuite, "session_recv_client_preface", !CU_add_test(pSuite, "session_recv_client_preface",
test_nghttp2_session_recv_client_preface) || test_nghttp2_session_recv_client_preface) ||
!CU_add_test(pSuite, "session_delete_data_item",
test_nghttp2_session_delete_data_item) ||
!CU_add_test(pSuite, "frame_pack_headers", !CU_add_test(pSuite, "frame_pack_headers",
test_nghttp2_frame_pack_headers) || test_nghttp2_frame_pack_headers) ||
!CU_add_test(pSuite, "frame_pack_headers_frame_too_large", !CU_add_test(pSuite, "frame_pack_headers_frame_too_large",
......
...@@ -6449,3 +6449,29 @@ void test_nghttp2_session_recv_client_preface(void) ...@@ -6449,3 +6449,29 @@ void test_nghttp2_session_recv_client_preface(void)
nghttp2_option_del(option); nghttp2_option_del(option);
} }
void test_nghttp2_session_delete_data_item(void)
{
nghttp2_session *session;
nghttp2_session_callbacks callbacks;
nghttp2_stream *a, *b;
nghttp2_data_provider prd;
memset(&callbacks, 0, sizeof(callbacks));
nghttp2_session_server_new(&session, &callbacks, NULL);
a = open_stream(session, 1);
b = open_stream_with_dep(session, 3, a);
/* We don't care about these members, since we won't send data */
prd.source.ptr = NULL;
prd.read_callback = fail_data_source_read_callback;
/* This data item will be marked as TOP */
CU_ASSERT(0 == nghttp2_submit_data(session, NGHTTP2_FLAG_NONE, 1, &prd));
/* This data item will be marked as REST */
CU_ASSERT(0 == nghttp2_submit_data(session, NGHTTP2_FLAG_NONE, 3, &prd));
nghttp2_session_del(session);
}
...@@ -114,5 +114,6 @@ void test_nghttp2_session_keep_closed_stream(void); ...@@ -114,5 +114,6 @@ void test_nghttp2_session_keep_closed_stream(void);
void test_nghttp2_session_graceful_shutdown(void); void test_nghttp2_session_graceful_shutdown(void);
void test_nghttp2_session_on_header_temporal_failure(void); void test_nghttp2_session_on_header_temporal_failure(void);
void test_nghttp2_session_recv_client_preface(void); void test_nghttp2_session_recv_client_preface(void);
void test_nghttp2_session_delete_data_item(void);
#endif /* NGHTTP2_SESSION_TEST_H */ #endif /* NGHTTP2_SESSION_TEST_H */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment