AVRO-132. Fix multi-threading race condition when threads share schem…

…a objects.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/avro/trunk@822968 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -70,6 +70,9 @@ Trunk (unreleased changes)
 
   BUG FIXES
 
+    AVRO-132.  Fix multi-threading race condition when threads share schema objects.
+    (sbanacho)
+
     AVRO-113.  Fix endian bug with C++ integer/long varint codec.
     (Scott Banachowski via cutting)
 

src/c++/api/Boost.hh

@@ -55,7 +55,7 @@ namespace boost {
 
     typedef integral_constant<bool, true>  true_type;
     typedef integral_constant<bool, false> false_type;
-}
+} // namespace boost
 #else 
 #include <boost/type_traits.hpp>
 #endif // AVRO_BOOST_NO_TRAIT
@@ -93,7 +93,7 @@ namespace boost {
       private:
         std::vector<T *> ptrs_;
     };
-}
+} // namespace boost
 #else 
 #include <boost/ptr_container/ptr_vector.hpp>
 #endif // AVRO_BOOST_NO_PTRVECTOR

src/c++/api/Node.hh

@@ -21,7 +21,7 @@
 
 #include <cassert>
 #include <boost/noncopyable.hpp>
-#include <boost/intrusive_ptr.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include "Exception.hh"
 #include "Types.hh"
@@ -30,7 +30,7 @@ namespace avro {
 
 class Node;
 
-typedef boost::intrusive_ptr<Node> NodePtr;
+typedef boost::shared_ptr<Node> NodePtr;
 
 
 /// Node is the building block for parse trees.  Each node represents an avro
@@ -40,10 +40,9 @@ typedef boost::intrusive_ptr<Node> NodePtr;
 /// The user does not use the Node object directly, they interface with Schema
 /// objects.
 ///
-/// The Node object has an embedded reference count for use in the
-/// boost::intrusive_ptr smart pointer.  This is so that schemas may be reused
-/// in other other schemas, without needing to worry about memory deallocation
-/// for nodes that are added to multiple schema parse trees.
+/// The Node object uses reference-counted pointers.  This is so that schemas 
+/// may be reused in other other schemas, without needing to worry about memory
+/// deallocation for nodes that are added to multiple schema parse trees.
 ///
 /// Node has minimal implementation, serving as an abstract base class for
 /// different node types.
@@ -130,27 +129,11 @@ class Node : private boost::noncopyable
 
   private:
 
-    friend void intrusive_ptr_add_ref(Node *node);
-    friend void intrusive_ptr_release(Node *node);
-
     const Type type_;
     int refCount_;
     bool locked_;
 };
 
-inline void 
-intrusive_ptr_add_ref(Node *node) {
-    ++(node->refCount_);
-}
-
-inline void 
-intrusive_ptr_release(Node *node) {
-    --(node->refCount_);
-    if(node->refCount_ == 0) { 
-        delete node;
-    }
-}
-
 } // namespace avro
 
 #endif

src/c++/api/ValidSchema.hh

@@ -52,9 +52,9 @@ class ValidSchema : private boost::noncopyable
         return node_;
     }
 
-    void toJson(std::ostream &os);
+    void toJson(std::ostream &os) const;
 
-    void toFlatList(std::ostream &os);
+    void toFlatList(std::ostream &os) const;
 
     NodePtr followSymbol(const std::string &name) const {
         return symbolMap_.locateSymbol(name);

src/c++/api/ValidatingReader.hh

@@ -47,7 +47,7 @@ class ValidatingReader : private boost::noncopyable
 
   public:
 
-    explicit ValidatingReader(const ValidSchema &schema, InputStreamer &in);
+    ValidatingReader(const ValidSchema &schema, InputStreamer &in);
 
     template<typename T>
     void getValue(T &val) {

src/c++/api/Validator.hh

@@ -42,7 +42,7 @@ class Validator : private boost::noncopyable
 
   public:
 
-    Validator(const ValidSchema &schema);
+    explicit Validator(const ValidSchema &schema);
 
     void advance();
     void advanceWithCount(int64_t val);
@@ -80,7 +80,12 @@ class Validator : private boost::noncopyable
     void setupFlag(Type type);
 
     const ValidSchema &schema_;
-    NodePtr parseTree_;
+
+    // since this only keeps a reference to the schema, to ensure its parse
+    // tree is not deleted, keep a copy of a shared pointer to the root of the
+    // tree
+
+    const NodePtr parseTree_;
 
     Type nextType_; 
     flag_t expectedTypesFlag_;

src/c++/impl/NodeImpl.cc

@@ -159,36 +159,36 @@ nodeFromCompilerNode(CompilerNode &node)
     switch(node.type()) {
 
       case AVRO_ARRAY:
-        ptr = ( new NodeArray(node));
+        ptr.reset ( new NodeArray(node));
         break;
 
       case AVRO_ENUM:
-        ptr = ( new NodeEnum(node));
+        ptr.reset ( new NodeEnum(node));
         break;
 
       case AVRO_FIXED:
-        ptr = ( new NodeFixed(node));
+        ptr.reset ( new NodeFixed(node));
         break;
 
       case AVRO_MAP:
-        ptr = ( new NodeMap(node));
+        ptr.reset ( new NodeMap(node));
         break;
 
       case AVRO_RECORD:
-        ptr = ( new NodeRecord(node));
+        ptr.reset ( new NodeRecord(node));
         break;
 
       case AVRO_UNION:
-        ptr = ( new NodeUnion(node));
+        ptr.reset ( new NodeUnion(node));
         break;
 
       case AVRO_SYMBOLIC:
-        ptr = ( new NodeSymbolic(node));
+        ptr.reset ( new NodeSymbolic(node));
         break;
 
       default:
         if(isPrimitive(node.type())) {
-            ptr = ( new NodePrimitive(node.type()));        
+            ptr.reset ( new NodePrimitive(node.type()));        
         }
         else {
             throw Exception("Unknown type in nodeFromCompilerNode");

src/c++/impl/ValidSchema.cc

@@ -46,7 +46,7 @@ bool
 ValidSchema::validate(const NodePtr &node) 
 {
     if(!node) {
-        node_ = new NodePrimitive(AVRO_NULL);
+        node_.reset(new NodePrimitive(AVRO_NULL));
     }
 
     if(!node->isValid()) {
@@ -78,14 +78,14 @@ ValidSchema::validate(const NodePtr &node)
 }
 
 void 
-ValidSchema::toJson(std::ostream &os)
+ValidSchema::toJson(std::ostream &os) const
 { 
     node_->printJson(os, 0);
     os << '\n';
 }
 
 void 
-ValidSchema::toFlatList(std::ostream &os)
+ValidSchema::toFlatList(std::ostream &os) const
 { 
     node_->printBasicInfo(os);
 }

src/c++/impl/ValidatingReader.cc

@@ -78,4 +78,4 @@ ValidatingReader::getArrayBlockSize()
     return getCount();
 }
 
-} // namepspace avro
+} // namespace avro

src/c++/impl/ValidatingWriter.cc

@@ -98,4 +98,4 @@ ValidatingWriter::beginEnum(int64_t choice)
 }
 
 
-} // namepspace avro
+} // namespace avro

src/c++/impl/Validator.cc

@@ -295,4 +295,4 @@ Validator::getNextFieldName(std::string &name) const
     return found;
 }
 
-} // namepspace avro
+} // namespace avro