Repository: trafficserver Updated Branches: refs/heads/master 05f4db0d9 -> 1fec97e14
Adding unit test for HdrHeap coalesce crash (TS-2766) Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/1fec97e1 Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/1fec97e1 Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/1fec97e1 Branch: refs/heads/master Commit: 1fec97e14491d9ecd2f90063ceeaea8dcea97a8e Parents: 05f4db0 Author: Brian Geffon <[email protected]> Authored: Mon May 19 15:33:37 2014 -0700 Committer: Brian Geffon <[email protected]> Committed: Mon May 19 15:33:37 2014 -0700 ---------------------------------------------------------------------- proxy/hdrs/HdrHeap.cc | 124 +++++++++++++++++++++++++++------------------ 1 file changed, 76 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/1fec97e1/proxy/hdrs/HdrHeap.cc ---------------------------------------------------------------------- diff --git a/proxy/hdrs/HdrHeap.cc b/proxy/hdrs/HdrHeap.cc index 296f766..cd762e6 100644 --- a/proxy/hdrs/HdrHeap.cc +++ b/proxy/hdrs/HdrHeap.cc @@ -1198,68 +1198,96 @@ struct StrTest int len; }; -#ifdef TESTS -static void -hdr_heap_test_verify(StrTest * str_test, int n) -{ - for (int i = 0; i < n; i++) { - char *ptr = str_test[i].ptr; - int len = str_test[i].len; - - char c = (char) (len % 43); - for (int m = 0; m < len; m++) { - if (ptr[m] != c) { - Warning("Str Test failed on str %d of %d", i, n); - return; - } - c++; - } +#if TS_HAS_TESTS +#include <ts/TestBox.h> +REGRESSION_TEST(HdrHeap_Coalesce)(RegressionTest* t, int /* atype ATS_UNUSED */, int* pstatus) { + *pstatus = REGRESSION_TEST_PASSED; + /* + * This test is designed to test numerous pieces of the HdrHeaps including allocations, + * demotion of rw heaps to ronly heaps, and finally the coalesce and evacuate behaviours. + */ + + // The amount of space we will need to overflow the StrHdrHeap is HDR_STR_HEAP_DEFAULT_SIZE - STR_HEAP_HDR_SIZE + size_t next_rw_heap_size = HDR_STR_HEAP_DEFAULT_SIZE; + size_t next_required_overflow_size = next_rw_heap_size - STR_HEAP_HDR_SIZE; + char buf[next_required_overflow_size]; + for(unsigned int i = 0; i < sizeof(buf); ++i) { + buf[i] = ('a' + (i % 26)); } -} + TestBox tb(t, pstatus); + HdrHeap *heap = new_HdrHeap(); + URLImpl *url = url_create(heap); -// NOTE: the tests are current broken since -// they don't have objects that can -// string evacuation -void -hdr_heap_test() -{ - Note("Starting hdr heap test"); + tb.check(heap->m_read_write_heap.m_ptr == NULL, "Checking that we have no rw heap."); + url_path_set(heap,url, buf, next_required_overflow_size, true); + tb.check(heap->m_read_write_heap->m_free_size == 0, "Checking that we've completely consumed the rw heap"); + for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) { + tb.check(heap->m_ronly_heap[i].m_heap_start == (char*)NULL, "Checking ronly_heap[%d] is NULL", i); + } + + // Now we have no ronly heaps in use and a completely full rwheap, so we will test that + // we demote to ronly heaps HDR_BUF_RONLY_HEAPS times. + for (int ronly_heap = 0; ronly_heap < HDR_BUF_RONLY_HEAPS; ++ronly_heap) { + next_rw_heap_size = 2 * heap->m_read_write_heap->m_heap_size; + next_required_overflow_size = next_rw_heap_size - STR_HEAP_HDR_SIZE; + char buf2[next_required_overflow_size]; + for(unsigned int i = 0; i < sizeof(buf2); ++i) { + buf2[i] = ('a' + (i % 26)); + } - HdrHeap *h = new_HdrHeap(); + URLImpl *url2 = url_create(heap); + url_path_set(heap,url2,buf2,next_required_overflow_size, true); - int s; - for (int i = 0; i < 10; i++) { - s = 16; - while (s < HDR_MAX_ALLOC_SIZE) { - h->allocate_obj(s); - s++; + tb.check(heap->m_read_write_heap->m_heap_size == (uint32_t)next_rw_heap_size, "Checking the current rw heap is %d bytes", next_rw_heap_size); + tb.check(heap->m_read_write_heap->m_free_size == 0, "Checking that we've completely consumed the rw heap"); + tb.check(heap->m_ronly_heap[ronly_heap].m_heap_start != NULL, "Checking that we properly demoted the previous rw heap"); + for (int i = ronly_heap + 1; i < HDR_BUF_RONLY_HEAPS; ++i) { + tb.check(heap->m_ronly_heap[i].m_heap_start == NULL, "Checking ronly_heap[%d] is NULL", i); } } + // We will rerun these checks after we introduce a non-copied string to make sure we didn't already coalesce + for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) { + tb.check(heap->m_ronly_heap[i].m_heap_start != (char*)NULL, "Pre non-copied string: Checking ronly_heap[%d] is NOT NULL", i); + } - // Allocate a bunch of strings - StrTest str_test[10000]; - s = 16; - for (int j = 0; j < 10000; j++) { - str_test[j].ptr = h->allocate_str(s); - str_test[j].len = s; + // Now if we add a url object that contains only non-copied strings it shouldn't affect the size of the rwheap + // since it doesn't require allocating any storage on this heap. + char buf3[next_required_overflow_size]; + for(unsigned int i = 0; i < sizeof(buf); ++i) { + buf3[i] = ('a' + (i % 26)); + } - char c = (char) (s % 43); - for (int k = 0; k < s; k++) { - str_test[j].ptr[k] = c; - c++; - } + URLImpl *aliased_str_url = url_create(heap); + url_path_set(heap, aliased_str_url, buf3, next_required_overflow_size, false); // don't copy this string + tb.check(aliased_str_url->m_len_path == next_required_overflow_size, "Checking that the aliased string shows having proper length"); + tb.check(aliased_str_url->m_ptr_path == buf3, "Checking that the aliased string is correctly pointing at buf"); - hdr_heap_test_verify(str_test, j + 1); - s += 1; + + for (int i = 0; i < HDR_BUF_RONLY_HEAPS; ++i) { + tb.check(heap->m_ronly_heap[i].m_heap_start != (char*)NULL, "Post non-copied string: Checking ronly_heap[%d] is NOT NULL", i); } + tb.check(heap->m_read_write_heap->m_free_size == 0, "Checking that we've completely consumed the rw heap"); + tb.check(heap->m_next == NULL, "Checking that we dont have any chained heaps"); + // Now at this point we have a completely full rw_heap and no ronly heap slots, so any allocation would have to result + // in a coalesce, and to validate that we don't reintroduce TS-2766 we have an aliased string, so when it tries to + // coalesce it used to sum up the size of the ronly heaps and the rw heap which is incorrect because we never + // copied the above string onto the heap. The new behaviour fixed in TS-2766 will make sure that this non copied + // string is accounted for, in the old implementation it would result in an allocation failure. - // Make sure the strings are all intact - hdr_heap_test_verify(str_test, 100); - Note("Str Test completed"); -} + char *str = heap->allocate_str(1); // this will force a coalese. + tb.check(str != NULL, "Checking that 1 byte allocated string is not NULL"); + // Now we need to validate that aliased_str_url has a path that isn't NULL, if it's NULL then the + // coalesce is broken and didn't properly determine the size, if it's not null then everything worked as expected. + tb.check(aliased_str_url->m_len_path == next_required_overflow_size, "Checking that the aliased string shows having proper length"); + tb.check(aliased_str_url->m_ptr_path != NULL, "Checking that the aliased string was properly moved during coalsece and evacuation"); + tb.check(aliased_str_url->m_ptr_path != buf3, "Checking that the aliased string was properly moved during coalsece and evacuation (not pointing at buf3)"); + + // Clean up + heap->destroy(); +} #endif
