From 31de732e3b34e60a50d5cc1ee88884d4768f4a4c Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 22 Jun 2014 13:39:17 +0900 Subject: [PATCH] Allocate header table ringbuffer lazily Previously in inflater we reserve new ringbuffer when table size is changed. This may be potentially a problem if new table size is very large number. When inflater is not used directly by application, this is not a problem because application can choose the buffer size. On the other hand, if application uses inflater directly and it does not have control of new buffer size (e.g., protocol dissector), then we just fail to allocate large buffer in nghttp2_hd_inflate_change_table_size() without actually use such huge buffer. This change defers the actual allocation of buffer when it is actually needed so that we will fail when it is absolutely needed. --- lib/nghttp2_hd.c | 39 ++++++++++++++------------ tests/main.c | 2 ++ tests/nghttp2_hd_test.c | 62 +++++++++++++++++++++++++++++++++-------- tests/nghttp2_hd_test.h | 1 + 4 files changed, 75 insertions(+), 29 deletions(-) diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 33676efb..6161261a 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -273,12 +273,20 @@ static void hd_ringbuf_free(nghttp2_hd_ringbuf *ringbuf) free(ringbuf->buffer); } -static size_t hd_ringbuf_push_front(nghttp2_hd_ringbuf *ringbuf, - nghttp2_hd_entry *ent) +static int hd_ringbuf_push_front(nghttp2_hd_ringbuf *ringbuf, + nghttp2_hd_entry *ent) { - assert(ringbuf->len <= ringbuf->mask); + int rv; + + rv = hd_ringbuf_reserve(ringbuf, ringbuf->len + 1); + + if(rv != 0) { + return rv; + } + ringbuf->buffer[--ringbuf->first & ringbuf->mask] = ent; ++ringbuf->len; + return 0; } @@ -852,8 +860,17 @@ static nghttp2_hd_entry* add_hd_table_incremental(nghttp2_hd_context *context, immediately evicted. */ --new_ent->ref; } else { + rv = hd_ringbuf_push_front(&context->hd_table, new_ent); + + if(rv != 0) { + --new_ent->ref; + nghttp2_hd_entry_free(new_ent); + free(new_ent); + + return NULL; + } + context->hd_table_bufsize += room; - hd_ringbuf_push_front(&context->hd_table, new_ent); new_ent->flags |= NGHTTP2_HD_FLAG_REFSET; } @@ -950,14 +967,8 @@ static void hd_context_shrink_table_size(nghttp2_hd_context *context) int nghttp2_hd_deflate_change_table_size(nghttp2_hd_deflater *deflater, size_t settings_hd_table_bufsize_max) { - int rv; size_t next_bufsize = nghttp2_min(settings_hd_table_bufsize_max, deflater->deflate_hd_table_bufsize_max); - rv = hd_ringbuf_reserve - (&deflater->ctx.hd_table, next_bufsize / NGHTTP2_HD_ENTRY_OVERHEAD); - if(rv != 0) { - return rv; - } deflater->ctx.hd_table_bufsize_max = next_bufsize; @@ -970,14 +981,6 @@ int nghttp2_hd_deflate_change_table_size(nghttp2_hd_deflater *deflater, int nghttp2_hd_inflate_change_table_size(nghttp2_hd_inflater *inflater, size_t settings_hd_table_bufsize_max) { - int rv; - - rv = hd_ringbuf_reserve - (&inflater->ctx.hd_table, - settings_hd_table_bufsize_max / NGHTTP2_HD_ENTRY_OVERHEAD); - if(rv != 0) { - return rv; - } inflater->settings_hd_table_bufsize_max = settings_hd_table_bufsize_max; inflater->ctx.hd_table_bufsize_max = settings_hd_table_bufsize_max; hd_context_shrink_table_size(&inflater->ctx); diff --git a/tests/main.c b/tests/main.c index 659d19b9..042a86be 100644 --- a/tests/main.c +++ b/tests/main.c @@ -273,6 +273,8 @@ int main(int argc, char* argv[]) test_nghttp2_hd_inflate_clearall_inc) || !CU_add_test(pSuite, "hd_inflate_zero_length_huffman", test_nghttp2_hd_inflate_zero_length_huffman) || + !CU_add_test(pSuite, "hd_ringbuf_reserve", + test_nghttp2_hd_ringbuf_reserve) || !CU_add_test(pSuite, "hd_change_table_size", test_nghttp2_hd_change_table_size) || !CU_add_test(pSuite, "hd_deflate_inflate", diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index 62ea279a..f2309d31 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -641,6 +641,57 @@ void test_nghttp2_hd_inflate_zero_length_huffman(void) nghttp2_hd_inflate_free(&inflater); } +void test_nghttp2_hd_ringbuf_reserve(void) +{ + nghttp2_hd_deflater deflater; + nghttp2_hd_inflater inflater; + nghttp2_nv nv; + nghttp2_bufs bufs; + nva_out out; + int i; + ssize_t rv; + ssize_t blocklen; + + frame_pack_bufs_init(&bufs); + nva_out_init(&out); + + nv.flags = NGHTTP2_NV_FLAG_NONE; + nv.name = (uint8_t*)"a"; + nv.namelen = strlen((const char*)nv.name); + nv.valuelen = 4; + nv.value = malloc(nv.valuelen); + memset(nv.value, 0, nv.valuelen); + + nghttp2_hd_deflate_init2(&deflater, 8000); + nghttp2_hd_inflate_init(&inflater); + + nghttp2_hd_inflate_change_table_size(&inflater, 8000); + nghttp2_hd_deflate_change_table_size(&deflater, 8000); + + for(i = 0; i < 150; ++i) { + memcpy(nv.value, &i, sizeof(i)); + rv = nghttp2_hd_deflate_hd_bufs(&deflater, &bufs, &nv, 1); + blocklen = nghttp2_bufs_len(&bufs); + + CU_ASSERT(0 == rv); + CU_ASSERT(blocklen > 0); + + CU_ASSERT(blocklen == inflate_hd(&inflater, &out, &bufs, 0)); + + CU_ASSERT(1 == out.nvlen); + assert_nv_equal(&nv, out.nva, 1); + + nva_out_reset(&out); + nghttp2_bufs_reset(&bufs); + } + + nghttp2_bufs_free(&bufs); + nghttp2_hd_inflate_free(&inflater); + nghttp2_hd_deflate_free(&deflater); + + free(nv.value); +} + void test_nghttp2_hd_change_table_size(void) { nghttp2_hd_deflater deflater; @@ -663,10 +714,8 @@ void test_nghttp2_hd_change_table_size(void) CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, 8000)); CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, 8000)); - CU_ASSERT(127 == deflater.ctx.hd_table.mask); CU_ASSERT(4096 == deflater.ctx.hd_table_bufsize_max); - CU_ASSERT(255 == inflater.ctx.hd_table.mask); CU_ASSERT(8000 == inflater.ctx.hd_table_bufsize_max); CU_ASSERT(8000 == inflater.settings_hd_table_bufsize_max); @@ -691,10 +740,8 @@ void test_nghttp2_hd_change_table_size(void) CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, 1024)); CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, 1024)); - CU_ASSERT(127 == deflater.ctx.hd_table.mask); CU_ASSERT(1024 == deflater.ctx.hd_table_bufsize_max); - CU_ASSERT(255 == inflater.ctx.hd_table.mask); CU_ASSERT(1024 == inflater.ctx.hd_table_bufsize_max); CU_ASSERT(1024 == inflater.settings_hd_table_bufsize_max); @@ -718,11 +765,9 @@ void test_nghttp2_hd_change_table_size(void) CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, 0)); CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, 0)); - CU_ASSERT(127 == deflater.ctx.hd_table.mask); CU_ASSERT(0 == deflater.ctx.hd_table.len); CU_ASSERT(0 == deflater.ctx.hd_table_bufsize_max); - CU_ASSERT(255 == inflater.ctx.hd_table.mask); CU_ASSERT(0 == inflater.ctx.hd_table.len); CU_ASSERT(0 == inflater.ctx.hd_table_bufsize_max); CU_ASSERT(0 == inflater.settings_hd_table_bufsize_max); @@ -757,10 +802,8 @@ void test_nghttp2_hd_change_table_size(void) CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, 8000)); CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, 8000)); - CU_ASSERT(255 == deflater.ctx.hd_table.mask); CU_ASSERT(8000 == deflater.ctx.hd_table_bufsize_max); - CU_ASSERT(255 == inflater.ctx.hd_table.mask); CU_ASSERT(8000 == inflater.ctx.hd_table_bufsize_max); CU_ASSERT(8000 == inflater.settings_hd_table_bufsize_max); @@ -783,10 +826,8 @@ void test_nghttp2_hd_change_table_size(void) CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, 16383)); CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, 16383)); - CU_ASSERT(255 == deflater.ctx.hd_table.mask); CU_ASSERT(8192 == deflater.ctx.hd_table_bufsize_max); - CU_ASSERT(511 == inflater.ctx.hd_table.mask); CU_ASSERT(16383 == inflater.ctx.hd_table_bufsize_max); CU_ASSERT(16383 == inflater.settings_hd_table_bufsize_max); @@ -823,7 +864,6 @@ void test_nghttp2_hd_change_table_size(void) nghttp2_hd_deflate_init2(&deflater, 1024); nghttp2_hd_inflate_init(&inflater); - CU_ASSERT(127 == deflater.ctx.hd_table.mask); CU_ASSERT(1024 == deflater.ctx.hd_table_bufsize_max); /* This emits context update with buffer size 1024 */ diff --git a/tests/nghttp2_hd_test.h b/tests/nghttp2_hd_test.h index 237751ba..51bf4ed6 100644 --- a/tests/nghttp2_hd_test.h +++ b/tests/nghttp2_hd_test.h @@ -37,6 +37,7 @@ void test_nghttp2_hd_inflate_newname_noinc(void); void test_nghttp2_hd_inflate_newname_inc(void); void test_nghttp2_hd_inflate_clearall_inc(void); void test_nghttp2_hd_inflate_zero_length_huffman(void); +void test_nghttp2_hd_ringbuf_reserve(void); void test_nghttp2_hd_change_table_size(void); void test_nghttp2_hd_deflate_inflate(void); void test_nghttp2_hd_no_index(void);