diff options
| author | Nat Goodspeed <nat@lindenlab.com> | 2019-06-27 10:57:34 -0400 |
|---|---|---|
| committer | Nat Goodspeed <nat@lindenlab.com> | 2020-03-25 18:44:04 -0400 |
| commit | 2902f23a4193d93c2e96daa45587a8c597c0a831 (patch) | |
| tree | 96655824fd61c58335a388418b3a9147a1ce80fc /indra/llcorehttp/tests/test_bufferarray.hpp | |
| parent | 3a7d40136415fbf1897d5240ef4e4d48f79c8a26 (diff) | |
DRTVWR-476: Remove special llcorehttp test memory manager.
NickyD discovered that the substitute default allocator used for llcorehttp
tests was returning badly-aligned storage, which caused access violations on
alignment-sensitive data such as std::atomic. Thanks Nicky!!
Moreover, the llcorehttp test assertions regarding memory usage, well-
intentioned though they are, have been causing us trouble for years. Many have
already been disabled.
The problem is that use of test_allocator.h affected *everything* defined with
that header file's declarations visible. That inevitably included specific
functions in other subsystems. Those functions then (unintentionally) consumed
the special allocator, throwing off the memory tracking and making certain
memory-related assertions consistently fail.
This is a particular, observable bad effect of One Definition Rule violations.
Within a given program, C++ allows multiple definitions for the same entity,
but requires that all such definitions be the same. Partial visibility of the
global operator new() and operator delete() overrides meant that some
definitions of certain entities used the default global allocator, some used
llcorehttp's. There may have been other, more subtle bad effects of these ODR
violations.
If one wanted to reimplement verification of the memory consumption of
llcorehttp classes:
* Each llcorehttp class (for which memory tracking was desired) should declare
class-specific operator new() and operator delete() methods. Naturally,
these would all consume a central llcorehttp-specific allocator, but that
allocator should *not* be named global operator new().
* Presumably that would require runtime indirection to allow using the default
allocator in production while substituting the special allocator for tests.
* Recording and verifying the memory consumption in each test should be
performed in the test-object constructor and destructor, rather than being
sprinkled throughout the test<n>() methods.
* With that mechanism in place, the test object should provide methods to
adjust (or entirely disable) memory verification for a particular test.
* The test object should also provide a "yes, we're still consuming llcorehttp
memory" method to be used for spot checks in the middle of tests -- instead
of sprinkling in explicit comparisons as before.
* In fact, the llcorehttp test object in each test_*.hpp file should be
derived from a central llcorehttp test-object base class providing those
methods.
Diffstat (limited to 'indra/llcorehttp/tests/test_bufferarray.hpp')
| -rw-r--r-- | indra/llcorehttp/tests/test_bufferarray.hpp | 52 |
1 files changed, 0 insertions, 52 deletions
diff --git a/indra/llcorehttp/tests/test_bufferarray.hpp b/indra/llcorehttp/tests/test_bufferarray.hpp index 8a2a64d970..cc4ad2a906 100644 --- a/indra/llcorehttp/tests/test_bufferarray.hpp +++ b/indra/llcorehttp/tests/test_bufferarray.hpp @@ -30,8 +30,6 @@ #include <iostream> -#include "test_allocator.h" - using namespace LLCore; @@ -44,7 +42,6 @@ struct BufferArrayTestData { // the test objects inherit from this so the member functions and variables // can be referenced directly inside of the test functions. - size_t mMemTotal; }; typedef test_group<BufferArrayTestData> BufferArrayTestGroupType; @@ -56,13 +53,9 @@ void BufferArrayTestObjectType::test<1>() { set_test_name("BufferArray construction"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); ensure("One ref on construction of BufferArray", ba->getRefCount() == 1); - ensure("Memory being used", mMemTotal < GetMemTotal()); ensure("Nothing in BA", 0 == ba->size()); // Try to read @@ -72,9 +65,6 @@ void BufferArrayTestObjectType::test<1>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure(mMemTotal == GetMemTotal()); } template <> template <> @@ -82,9 +72,6 @@ void BufferArrayTestObjectType::test<2>() { set_test_name("BufferArray single write"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -105,9 +92,6 @@ void BufferArrayTestObjectType::test<2>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure(mMemTotal == GetMemTotal()); } @@ -116,9 +100,6 @@ void BufferArrayTestObjectType::test<3>() { set_test_name("BufferArray multiple writes"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -154,9 +135,6 @@ void BufferArrayTestObjectType::test<3>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure(mMemTotal == GetMemTotal()); } template <> template <> @@ -164,9 +142,6 @@ void BufferArrayTestObjectType::test<4>() { set_test_name("BufferArray overwriting"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -208,9 +183,6 @@ void BufferArrayTestObjectType::test<4>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure(mMemTotal == GetMemTotal()); } template <> template <> @@ -218,9 +190,6 @@ void BufferArrayTestObjectType::test<5>() { set_test_name("BufferArray multiple writes - sequential reads"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -255,9 +224,6 @@ void BufferArrayTestObjectType::test<5>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure(mMemTotal == GetMemTotal()); } template <> template <> @@ -265,9 +231,6 @@ void BufferArrayTestObjectType::test<6>() { set_test_name("BufferArray overwrite spanning blocks and appending"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -306,9 +269,6 @@ void BufferArrayTestObjectType::test<6>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure("All memory released", mMemTotal == GetMemTotal()); } template <> template <> @@ -316,9 +276,6 @@ void BufferArrayTestObjectType::test<7>() { set_test_name("BufferArray overwrite spanning blocks and sequential writes"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -371,9 +328,6 @@ void BufferArrayTestObjectType::test<7>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure("All memory released", mMemTotal == GetMemTotal()); } template <> template <> @@ -381,9 +335,6 @@ void BufferArrayTestObjectType::test<8>() { set_test_name("BufferArray zero-length appendBufferAlloc"); - // record the total amount of dynamically allocated memory - mMemTotal = GetMemTotal(); - // create a new ref counted object with an implicit reference BufferArray * ba = new BufferArray(); @@ -421,9 +372,6 @@ void BufferArrayTestObjectType::test<8>() // release the implicit reference, causing the object to be released ba->release(); - - // make sure we didn't leak any memory - ensure("All memory released", mMemTotal == GetMemTotal()); } } // end namespace tut |
