From 16db1cb57501363fea65127cb5eee797399305ac Mon Sep 17 00:00:00 2001 From: Alan Conway Date: Mon, 20 Aug 2007 17:18:52 +0000 Subject: Fixed Blob bug causing test crashes/hangs. git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@567755 13f79535-47bb-0310-9956-ffa450edef68 --- cpp/src/qpid/framing/Blob.cpp | 1 + cpp/src/qpid/framing/Blob.h | 56 +++++++++------ cpp/src/qpid/framing/MethodHolder.cpp | 4 +- cpp/src/qpid/framing/MethodHolder.h | 2 +- cpp/src/tests/Blob | Bin 0 -> 223166 bytes cpp/src/tests/Blob.cpp | 125 ++++++++++++++++++++++++++++++++++ cpp/src/tests/Makefile.am | 13 ++-- 7 files changed, 171 insertions(+), 30 deletions(-) create mode 100755 cpp/src/tests/Blob create mode 100644 cpp/src/tests/Blob.cpp (limited to 'cpp/src') diff --git a/cpp/src/qpid/framing/Blob.cpp b/cpp/src/qpid/framing/Blob.cpp index 0aaeb4138e..388d4b64ef 100644 --- a/cpp/src/qpid/framing/Blob.cpp +++ b/cpp/src/qpid/framing/Blob.cpp @@ -25,6 +25,7 @@ namespace qpid { namespace framing { void BlobHelper::destroy(void*) {} + void BlobHelper::copy(void*, const void*) {} }} // namespace qpid::framing diff --git a/cpp/src/qpid/framing/Blob.h b/cpp/src/qpid/framing/Blob.h index ea44dc104e..9d0c33dee6 100644 --- a/cpp/src/qpid/framing/Blob.h +++ b/cpp/src/qpid/framing/Blob.h @@ -21,6 +21,7 @@ * */ +#include #include #include #include @@ -80,47 +81,62 @@ template <> struct BlobHelper { * Objects can be allocated directly in place using * construct(in_place(...)) or copied using operator=. * Constructing a new object in the blob destroys the old one. + * + * If BaseType is specified then only object that can be + * safely static_cast to BaseType may be stored in the Blob. */ -template +template class Blob { boost::aligned_storage store; + BaseType* basePtr; + void (*destroy)(void*); void (*copy)(void*, const void*); - template void setType() { + template void setType() { + BOOST_STATIC_ASSERT(sizeof(T) <= Size); destroy=&BlobHelper::destroy; copy=&BlobHelper::copy; + // Base pointer may be offeset from store.address() + basePtr = reinterpret_cast(store.address()); } - + + void initialize() { + destroy=&BlobHelper::destroy; + copy=&BlobHelper::copy; + basePtr=0; + } + template void construct (const TypedInPlaceFactory& factory, const boost::typed_in_place_factory_base* ) { - assert(empty()); typedef typename TypedInPlaceFactory::value_type T; - assert(sizeof(T) <= Size); + assert(empty()); factory.apply(store.address()); setType(); } void assign(const Blob& b) { assert(empty()); - b.copy(this->get(), b.get()); + b.copy(this->store.address(), b.store.address()); copy = b.copy; destroy = b.destroy; + basePtr = reinterpret_cast( + ((char*)this)+ ((char*)(b.basePtr) - (char*)(&b))); } - + public: /** Construct an empty blob. */ - Blob() { setType(); } + Blob() { initialize(); } /** Copy a blob. */ - Blob(const Blob& b) { setType(); assign(b); } + Blob(const Blob& b) { initialize(); assign(b); } /** @see construct() */ template - Blob( const Expr & expr ) { setType(); construct(expr,&expr); } + Blob( const Expr & expr ) { initialize(); construct(expr,&expr); } ~Blob() { clear(); } @@ -139,30 +155,28 @@ class Blob construct(const Expr& expr) { clear(); construct(expr,&expr); } /** Copy construct an instance of T into the Blob. */ - template + template Blob& operator=(const T& x) { clear(); construct(in_place(x)); return *this; } - - /** Get pointer to blob contents. Caller must know how to cast it. */ - void* get() { return store.address(); } - /** Get const pointer to blob contents */ - const void* get() const { return empty() ? 0 : store.address(); } - + /** Get pointer to blob contents, returns 0 if empty. */ + BaseType* get() { return basePtr; } + + /** Get pointer to blob contents, returns 0 if empty. */ + const BaseType* get() const { return basePtr; } + /** Destroy the object in the blob making it empty. */ void clear() { void (*oldDestroy)(void*) = destroy; - setType(); + initialize(); oldDestroy(store.address()); } - bool empty() const { return destroy == BlobHelper::destroy; } + bool empty() const { return destroy==BlobHelper::destroy; } static size_t size() { return Size; } }; - }} // namespace qpid::framing - #endif /*!QPID_FRAMING_BLOB_H*/ diff --git a/cpp/src/qpid/framing/MethodHolder.cpp b/cpp/src/qpid/framing/MethodHolder.cpp index b1582dd571..691d556ade 100644 --- a/cpp/src/qpid/framing/MethodHolder.cpp +++ b/cpp/src/qpid/framing/MethodHolder.cpp @@ -32,11 +32,11 @@ namespace qpid { namespace framing { AMQMethodBody* MethodHolder::get() { - return static_cast(blob.get()); + return blob.get(); } const AMQMethodBody* MethodHolder::get() const { - return const_cast(this)->get(); + return blob.get(); } void MethodHolder::encode(Buffer& b) const { diff --git a/cpp/src/qpid/framing/MethodHolder.h b/cpp/src/qpid/framing/MethodHolder.h index a8bc8f2728..5324b34ccd 100644 --- a/cpp/src/qpid/framing/MethodHolder.h +++ b/cpp/src/qpid/framing/MethodHolder.h @@ -84,7 +84,7 @@ class MethodHolder bool empty() const { return blob.empty(); } private: - Blob blob; + Blob blob; class CopyVisitor; friend struct CopyVisitor; }; diff --git a/cpp/src/tests/Blob b/cpp/src/tests/Blob new file mode 100755 index 0000000000..05e67dc01f Binary files /dev/null and b/cpp/src/tests/Blob differ diff --git a/cpp/src/tests/Blob.cpp b/cpp/src/tests/Blob.cpp new file mode 100644 index 0000000000..96861a3670 --- /dev/null +++ b/cpp/src/tests/Blob.cpp @@ -0,0 +1,125 @@ +/* + * + * Copyright (c) 2006 The Apache Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "qpid/framing/Blob.h" + +#define BOOST_AUTO_TEST_MAIN +#include + +using namespace std; +using namespace qpid::framing; + +struct Base { + int id; + int magic; + + Base(int n) : id(n), magic(42) {} + Base(const Base& c) : id(c.id), magic(42) {} + ~Base() { BOOST_CHECK_EQUAL(42, magic); } // Detect random data. +}; + +template struct Count : public Base { + static int instances; + bool destroyed; + + Count(int n) : Base(n), destroyed(false) { ++instances; } + Count(const Count& c) : Base(c), destroyed(false) { ++instances; } + ~Count() { + BOOST_CHECK(!destroyed); // Detect double-destructor + destroyed=true; + BOOST_CHECK(--instances >= 0); + } +}; + +template int Count::instances = 0; + +struct Foo : public Count { Foo(int n) : Count(n) {}; }; +struct Bar : public Count { Bar(int n) : Count(n) {}; }; + +typedef Blob TestBlob ; + +BOOST_AUTO_TEST_CASE(testCtor) { + { + TestBlob empty; + BOOST_CHECK(empty.empty()); + BOOST_CHECK(empty.get() == 0); + + TestBlob empty2(empty); + BOOST_CHECK(empty2.empty()); + + TestBlob foo(in_place(1)); + BOOST_CHECK(!foo.empty()); + BOOST_CHECK_EQUAL(1, foo.get()->id); + BOOST_CHECK_EQUAL(1, Foo::instances); + + TestBlob foo2(foo); + BOOST_CHECK(!foo2.empty()); + BOOST_CHECK_EQUAL(1, foo2.get()->id); + BOOST_CHECK_EQUAL(2, Foo::instances); + } + + BOOST_CHECK_EQUAL(0, Foo::instances); + BOOST_CHECK_EQUAL(0, Bar::instances); +} + + +BOOST_AUTO_TEST_CASE(testAssign) { + { + TestBlob b; + b = Foo(2); + BOOST_CHECK_EQUAL(2, b.get()->id); + BOOST_CHECK_EQUAL(1, Foo::instances); + + TestBlob b2(b); + BOOST_CHECK_EQUAL(2, b.get()->id); + BOOST_CHECK_EQUAL(2, Foo::instances); + + b2 = Bar(3); + BOOST_CHECK_EQUAL(3, b2.get()->id); + BOOST_CHECK_EQUAL(1, Foo::instances); + BOOST_CHECK_EQUAL(1, Bar::instances); + + b2.construct(in_place(4)); + BOOST_CHECK_EQUAL(4, b2.get()->id); + BOOST_CHECK_EQUAL(2, Foo::instances); + BOOST_CHECK_EQUAL(0, Bar::instances); + + b2.clear(); + BOOST_CHECK(b2.empty()); + BOOST_CHECK_EQUAL(1, Foo::instances); + } + BOOST_CHECK_EQUAL(0, Foo::instances); + BOOST_CHECK_EQUAL(0, Bar::instances); +} + + +BOOST_AUTO_TEST_CASE(testClear) { + TestBlob b(in_place(5)); + TestBlob c(b); + BOOST_CHECK(!c.empty()); + BOOST_CHECK(!b.empty()); + BOOST_CHECK_EQUAL(2, Foo::instances); + + c.clear(); + BOOST_CHECK(c.empty()); + BOOST_CHECK_EQUAL(1, Foo::instances); + + b.clear(); + BOOST_CHECK(b.empty()); + BOOST_CHECK_EQUAL(0, Foo::instances); +} diff --git a/cpp/src/tests/Makefile.am b/cpp/src/tests/Makefile.am index c9a08e6075..db9da392ea 100644 --- a/cpp/src/tests/Makefile.am +++ b/cpp/src/tests/Makefile.am @@ -19,6 +19,11 @@ CLEANFILES= # # Unit test programs. # +TESTS+=Blob +check_PROGRAMS+=Blob +Blob_SOURCES=Blob.cpp ../qpid/framing/Blob.cpp +Blob_LDADD=-lboost_unit_test_framework + TESTS+=logging check_PROGRAMS+=logging logging_SOURCES=logging.cpp test_tools.h @@ -120,13 +125,14 @@ check_PROGRAMS += $(testprogs) interop_runner TESTS_ENVIRONMENT = VALGRIND=$(VALGRIND) srcdir=$(srcdir) $(srcdir)/run_test -system_tests = client_test exception_test quick_topictest +system_tests = client_test exception_test quick_perftest quick_topictest TESTS += run-unit-tests start_broker $(system_tests) python_tests stop_broker EXTRA_DIST += \ test_env run_test vg_check \ run-unit-tests start_broker python_tests stop_broker \ quick_topictest \ + quick_perftest \ topictest \ .valgrind.supp-default \ .valgrindrc-default \ @@ -172,11 +178,6 @@ all-am: .valgrind.supp .valgrindrc .valgrind.supp: .valgrind.supp-default cp $^ $@ -# Tell GNU make not to build targets in this directory in parallel. -# This is necessary because with two or more identical and simultaneous -# ltmain invocations, one may corrupt the temporaries of the other. -.NOTPARALLEL: - CLEANFILES+=valgrind.out *.log *.vglog .valgrindrc .valgrind.supp dummy_test $(unit_wrappers) MAINTAINERCLEANFILES=gen.mk -- cgit v1.2.1