Fixed Blob bug causing test crashes/hangs.

git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk@567755 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 171 insertions, 30 deletions

diff --git a/qpid/cpp/src/qpid/framing/Blob.cpp b/qpid/cpp/src/qpid/framing/Blob.cpp
index 0aaeb4138e..388d4b64ef 100644
--- a/qpid/cpp/src/qpid/framing/Blob.cpp
+++ b/qpid/cpp/src/qpid/framing/Blob.cpp
@@ -25,6 +25,7 @@ namespace qpid {
 namespace framing {
 
 void BlobHelper<void>::destroy(void*) {}
+
 void BlobHelper<void>::copy(void*, const void*) {}
 
 }} // namespace qpid::framing
diff --git a/qpid/cpp/src/qpid/framing/Blob.h b/qpid/cpp/src/qpid/framing/Blob.h
index ea44dc104e..9d0c33dee6 100644
--- a/qpid/cpp/src/qpid/framing/Blob.h
+++ b/qpid/cpp/src/qpid/framing/Blob.h
@@ -21,6 +21,7 @@
  *
  */
 
+#include <boost/static_assert.hpp>
 #include <boost/aligned_storage.hpp>
 #include <boost/checked_delete.hpp>
 #include <boost/utility/typed_in_place_factory.hpp>
@@ -80,47 +81,62 @@ template <> struct BlobHelper<void> {
  * Objects can be allocated directly in place using
  * construct(in_place<T>(...))  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 <size_t Size>
+template <size_t Size, class BaseType=void>
 class Blob
 {
     boost::aligned_storage<Size> store;
+    BaseType* basePtr;
+    
     void (*destroy)(void*);
     void (*copy)(void*, const void*);
 
-    template <class T> void setType() {
+    template <class T>void setType() {
+        BOOST_STATIC_ASSERT(sizeof(T) <= Size);
         destroy=&BlobHelper<T>::destroy;
         copy=&BlobHelper<T>::copy;
+        // Base pointer may be offeset from store.address()
+        basePtr = reinterpret_cast<T*>(store.address());
     }
-    
+        
+    void initialize() {
+        destroy=&BlobHelper<void>::destroy;
+        copy=&BlobHelper<void>::copy;
+        basePtr=0;
+    }
+
     template<class TypedInPlaceFactory>
     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<T>();
     }
 
     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<BaseType*>(
+            ((char*)this)+ ((char*)(b.basePtr) - (char*)(&b)));
     }
-    
+
   public:
     /** Construct an empty blob. */
-    Blob() { setType<void>(); }
+    Blob() { initialize(); }
 
     /** Copy a blob. */
-    Blob(const Blob& b) { setType<void>(); assign(b); }
+    Blob(const Blob& b) { initialize(); assign(b); }
 
     /** @see construct() */
     template<class Expr>
-    Blob( const Expr & expr ) { setType<void>(); 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<class T>
+    template <class T>
     Blob& operator=(const T& x) { clear(); construct(in_place<T>(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<void>();
+        initialize();
         oldDestroy(store.address());
     }
 
-    bool empty() const { return destroy == BlobHelper<void>::destroy; }
+    bool empty() const { return destroy==BlobHelper<void>::destroy; }
     
     static size_t size() { return Size; }
 };
 
-
 }} // namespace qpid::framing
 
 
-
 #endif  /*!QPID_FRAMING_BLOB_H*/
diff --git a/qpid/cpp/src/qpid/framing/MethodHolder.cpp b/qpid/cpp/src/qpid/framing/MethodHolder.cpp
index b1582dd571..691d556ade 100644
--- a/qpid/cpp/src/qpid/framing/MethodHolder.cpp
+++ b/qpid/cpp/src/qpid/framing/MethodHolder.cpp
@@ -32,11 +32,11 @@ namespace qpid {
 namespace framing {
 
 AMQMethodBody* MethodHolder::get() {
-    return static_cast<AMQMethodBody*>(blob.get());
+    return blob.get();
 }
 
 const AMQMethodBody* MethodHolder::get() const {
-    return const_cast<MethodHolder*>(this)->get();
+    return blob.get();
 }
 
 void MethodHolder::encode(Buffer& b) const {
diff --git a/qpid/cpp/src/qpid/framing/MethodHolder.h b/qpid/cpp/src/qpid/framing/MethodHolder.h
index a8bc8f2728..5324b34ccd 100644
--- a/qpid/cpp/src/qpid/framing/MethodHolder.h
+++ b/qpid/cpp/src/qpid/framing/MethodHolder.h
@@ -84,7 +84,7 @@ class MethodHolder
     bool empty() const { return blob.empty(); }
 
   private:
-    Blob<MAX_METHODBODY_SIZE> blob;
+    Blob<MAX_METHODBODY_SIZE, AMQMethodBody> blob;
     class CopyVisitor;
   friend struct CopyVisitor;
 };
diff --git a/qpid/cpp/src/tests/Blob b/qpid/cpp/src/tests/Blob
new file mode 100755
index 0000000000..05e67dc01f
--- /dev/null
+++ b/qpid/cpp/src/tests/Blob
diff --git a/qpid/cpp/src/tests/Blob.cpp b/qpid/cpp/src/tests/Blob.cpp
new file mode 100644
index 0000000000..96861a3670
--- /dev/null
+++ b/qpid/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 <boost/test/auto_unit_test.hpp>
+
+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 <class T> 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 <class T> int Count<T>::instances = 0;
+
+struct Foo : public Count<Foo> { Foo(int n) : Count<Foo>(n) {}; };
+struct Bar : public Count<Bar> { Bar(int n) : Count<Bar>(n) {}; };
+
+typedef Blob<sizeof(Foo), Base> 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<Foo>(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<Foo>(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<Foo>(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/qpid/cpp/src/tests/Makefile.am b/qpid/cpp/src/tests/Makefile.am
index c9a08e6075..db9da392ea 100644
--- a/qpid/cpp/src/tests/Makefile.am
+++ b/qpid/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