You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by sb...@apache.org on 2009/11/19 20:07:40 UTC
svn commit: r882251 - in /hadoop/avro/trunk: ./ src/c++/api/ src/c++/impl/
src/c++/test/
Author: sbanacho
Date: Thu Nov 19 19:07:39 2009
New Revision: 882251
URL: http://svn.apache.org/viewvc?rev=882251&view=rev
Log:
AVRO-210. Memory leak with recursive schemas when constructed by hand.
AVRO-211. Nested schema does not get parsed in C++.
(same patch fixed both problems due to related code overlap)
Modified:
hadoop/avro/trunk/CHANGES.txt
hadoop/avro/trunk/src/c++/api/Compiler.hh
hadoop/avro/trunk/src/c++/api/CompilerNode.hh
hadoop/avro/trunk/src/c++/api/NodeImpl.hh
hadoop/avro/trunk/src/c++/impl/Compiler.cc
hadoop/avro/trunk/src/c++/impl/CompilerNode.cc
hadoop/avro/trunk/src/c++/impl/ValidSchema.cc
hadoop/avro/trunk/src/c++/impl/Validator.cc
hadoop/avro/trunk/src/c++/test/testgen.cc
Modified: hadoop/avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/CHANGES.txt?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/CHANGES.txt (original)
+++ hadoop/avro/trunk/CHANGES.txt Thu Nov 19 19:07:39 2009
@@ -110,6 +110,11 @@
AVRO-194. C++ varint encoding buffer too small. (sbanacho)
+ AVRO-210. Memory leak with recursive schemas when constructed
+ by hand. (sbanacho)
+
+ AVRO-211. Nested schema does not get parsed in C++. (sbanacho)
+
Avro 1.2.0 (14 October 2009)
INCOMPATIBLE CHANGES
Modified: hadoop/avro/trunk/src/c++/api/Compiler.hh
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/api/Compiler.hh?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/api/Compiler.hh (original)
+++ hadoop/avro/trunk/src/c++/api/Compiler.hh Thu Nov 19 19:07:39 2009
@@ -25,7 +25,6 @@
#include "Types.hh"
#include "Node.hh"
#include "CompilerNode.hh"
-#include "SymbolMap.hh"
namespace avro {
@@ -89,7 +88,6 @@
NodePtr root_;
Stack stack_;
- SymbolMap symbolMap_;
};
class ValidSchema;
Modified: hadoop/avro/trunk/src/c++/api/CompilerNode.hh
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/api/CompilerNode.hh?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/api/CompilerNode.hh (original)
+++ hadoop/avro/trunk/src/c++/api/CompilerNode.hh Thu Nov 19 19:07:39 2009
@@ -43,8 +43,7 @@
FIELDS,
VALUES,
ITEMS,
- TYPES,
- SYMBOLIC
+ TYPES
};
CompilerNode() :
@@ -88,9 +87,6 @@
case TYPES:
typesAttribute_.add(node);
break;
- case SYMBOLIC:
- symbolicAttribute_.add(node);
- break;
default:
throw Exception("Can't add node if the attribute type is not set");
@@ -117,9 +113,6 @@
// attribute used by maps:
concepts::SingleAttribute<NodePtr> valuesAttribute_;
- // attribute used by symbolic names:
- concepts::SingleAttribute<NodePtr> symbolicAttribute_;
-
// attribute used by unions:
concepts::MultiAttribute<NodePtr> typesAttribute_;
Modified: hadoop/avro/trunk/src/c++/api/NodeImpl.hh
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/api/NodeImpl.hh?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/api/NodeImpl.hh (original)
+++ hadoop/avro/trunk/src/c++/api/NodeImpl.hh Thu Nov 19 19:07:39 2009
@@ -20,6 +20,7 @@
#define avro_NodeImpl_hh__
#include <limits>
+#include <boost/weak_ptr.hpp>
#include "Node.hh"
#include "NodeConcepts.hh"
@@ -87,7 +88,7 @@
void doAddName(const std::string &name) {
if(! nameIndex_.add(name, leafNameAttributes_.size())) {
- throw Exception("Cannot add duplicate names");
+ throw Exception(boost::format("Cannot add duplicate name: %1%") % name);
}
leafNameAttributes_.add(name);
}
@@ -141,7 +142,7 @@
typedef concepts::SingleAttribute<int> HasSize;
typedef NodeImpl< NoName, NoLeaves, NoLeafNames, NoSize > NodeImplPrimitive;
-typedef NodeImpl< HasName, SingleLeaf, NoLeafNames, NoSize > NodeImplSymbolic;
+typedef NodeImpl< HasName, NoLeaves, NoLeafNames, NoSize > NodeImplSymbolic;
typedef NodeImpl< HasName, MultiLeaves, LeafNames, NoSize > NodeImplRecord;
typedef NodeImpl< HasName, NoLeaves, LeafNames, NoSize > NodeImplEnum;
@@ -169,14 +170,16 @@
class NodeSymbolic : public NodeImplSymbolic
{
+ typedef boost::weak_ptr<Node> NodeWeakPtr;
+
public:
NodeSymbolic() :
NodeImplSymbolic(AVRO_SYMBOLIC)
{ }
- explicit NodeSymbolic(const HasName &name, const SingleLeaf &node) :
- NodeImplSymbolic(AVRO_SYMBOLIC, name, node, NoLeafNames(), NoSize())
+ explicit NodeSymbolic(const HasName &name) :
+ NodeImplSymbolic(AVRO_SYMBOLIC, name, NoLeaves(), NoLeafNames(), NoSize())
{ }
SchemaResolution resolve(const Node &reader) const;
@@ -187,6 +190,26 @@
return (nameAttribute_.size() == 1);
}
+ bool isSet() const {
+ return (actualNode_.lock() != 0);
+ }
+
+ NodePtr getNode() const {
+ NodePtr node = actualNode_.lock();
+ if(!node) {
+ throw Exception(boost::format("Could not follow symbol %1%") % name());
+ }
+ return node;
+ }
+
+ void setNode(const NodePtr &node) {
+ actualNode_ = node;
+ }
+
+ protected:
+
+ NodeWeakPtr actualNode_;
+
};
class NodeRecord : public NodeImplRecord
@@ -199,7 +222,13 @@
NodeRecord(const HasName &name, const MultiLeaves &fields, const LeafNames &fieldsNames) :
NodeImplRecord(AVRO_RECORD, name, fields, fieldsNames, NoSize())
- { }
+ {
+ for(size_t i=0; i < leafNameAttributes_.size(); ++i) {
+ if(!nameIndex_.add(leafNameAttributes_.get(i), i)) {
+ throw Exception(boost::format("Cannot add duplicate name: %1%") % leafNameAttributes_.get(i));
+ }
+ }
+ }
SchemaResolution resolve(const Node &reader) const;
@@ -224,8 +253,14 @@
NodeEnum(const HasName &name, const LeafNames &symbols) :
NodeImplEnum(AVRO_ENUM, name, NoLeaves(), symbols, NoSize())
- { }
-
+ {
+ for(size_t i=0; i < leafNameAttributes_.size(); ++i) {
+ if(!nameIndex_.add(leafNameAttributes_.get(i), i)) {
+ throw Exception(boost::format("Cannot add duplicate name: %1%") % leafNameAttributes_.get(i));
+ }
+ }
+ }
+
SchemaResolution resolve(const Node &reader) const;
void printJson(std::ostream &os, int depth) const;
@@ -342,15 +377,18 @@
if(!B::hasAttribute) {
throw Exception("Cannot change leaf node for nonexistent leaf");
}
- NodePtr symbol(new NodeSymbolic);
NodePtr &replaceNode = const_cast<NodePtr &>(leafAttributes_.get(index));
if(replaceNode->name() != node->name()) {
throw Exception("Symbolic name does not match the name of the schema it references");
}
- symbol->setName(node->name());
- symbol->addLeaf(node);
- replaceNode = symbol;
+
+ NodePtr symbol(new NodeSymbolic);
+ NodeSymbolic *ptr = static_cast<NodeSymbolic *> (symbol.get());
+
+ ptr->setName(node->name());
+ ptr->setNode(node);
+ replaceNode.swap(symbol);
}
template < class A, class B, class C, class D >
@@ -380,6 +418,16 @@
}
}
+
+inline NodePtr resolveSymbol(const NodePtr &node)
+{
+ if(node->type() != AVRO_SYMBOLIC) {
+ throw Exception("Only symbolic nodes may be resolved");
+ }
+ boost::shared_ptr<NodeSymbolic> symNode = boost::static_pointer_cast<NodeSymbolic>(node);
+ return symNode->getNode();
+}
+
} // namespace avro
#endif
Modified: hadoop/avro/trunk/src/c++/impl/Compiler.cc
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/impl/Compiler.cc?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/impl/Compiler.cc (original)
+++ hadoop/avro/trunk/src/c++/impl/Compiler.cc Thu Nov 19 19:07:39 2009
@@ -71,13 +71,6 @@
else {
stack_.back().addNode(node);
}
-
- if(node->hasName() && node->type() != AVRO_SYMBOLIC) {
- bool registered = symbolMap_.registerSymbol(node);
- if(!registered) {
- throw Exception(boost::format("Symbol %1% already exists") % node->name());
- }
- }
}
void
@@ -127,16 +120,8 @@
#ifdef DEBUG_VERBOSE
std::cerr << "Adding named type " << text_ << '\n';
#endif
-
- NodePtr node = symbolMap_.locateSymbol(text_);
- if(node) {
- stack_.back().setType(AVRO_SYMBOLIC);
- stack_.back().nameAttribute_.add(text_);
- stack_.back().symbolicAttribute_.add(node);
- }
- else {
- throw Exception(boost::format("Could not resolve symbolic name %1%") % text_);
- }
+ stack_.back().setType(AVRO_SYMBOLIC);
+ stack_.back().nameAttribute_.add(text_);
}
void
Modified: hadoop/avro/trunk/src/c++/impl/CompilerNode.cc
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/impl/CompilerNode.cc?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/impl/CompilerNode.cc (original)
+++ hadoop/avro/trunk/src/c++/impl/CompilerNode.cc Thu Nov 19 19:07:39 2009
@@ -54,9 +54,9 @@
break;
case AVRO_SYMBOLIC:
- ptr.reset ( new NodeSymbolic(node.nameAttribute_, node.symbolicAttribute_));
+ ptr.reset ( new NodeSymbolic(node.nameAttribute_));
break;
-
+
default:
if(isPrimitive(node.type())) {
ptr.reset ( new NodePrimitive(node.type()));
Modified: hadoop/avro/trunk/src/c++/impl/ValidSchema.cc
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/impl/ValidSchema.cc?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/impl/ValidSchema.cc (original)
+++ hadoop/avro/trunk/src/c++/impl/ValidSchema.cc Thu Nov 19 19:07:39 2009
@@ -60,7 +60,12 @@
if(!symbolMap.hasSymbol(node->name())) {
throw Exception( boost::format("Symbolic name \"%1%\" is unknown") % node->name());
}
- return true;
+
+ boost::shared_ptr<NodeSymbolic> symNode = boost::static_pointer_cast<NodeSymbolic>(node);
+
+ // if the symbolic link is already resolved, we return true,
+ // otherwise returning false will force it to be resolved
+ return symNode->isSet();
}
bool registered = symbolMap.registerSymbol(node);
if(!registered) {
Modified: hadoop/avro/trunk/src/c++/impl/Validator.cc
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/impl/Validator.cc?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/impl/Validator.cc (original)
+++ hadoop/avro/trunk/src/c++/impl/Validator.cc Thu Nov 19 19:07:39 2009
@@ -20,6 +20,7 @@
#include "Validator.hh"
#include "ValidSchema.hh"
+#include "NodeImpl.hh"
namespace avro {
@@ -243,9 +244,9 @@
nextType_ = node->type();
if(nextType_ == AVRO_SYMBOLIC) {
- NodePtr symNode(node->leafAt(0));
- assert(symNode);
- setupOperation(symNode);
+ NodePtr actualNode = resolveSymbol(node);
+ assert(actualNode);
+ setupOperation(actualNode);
return;
}
Modified: hadoop/avro/trunk/src/c++/test/testgen.cc
URL: http://svn.apache.org/viewvc/hadoop/avro/trunk/src/c%2B%2B/test/testgen.cc?rev=882251&r1=882250&r2=882251&view=diff
==============================================================================
--- hadoop/avro/trunk/src/c++/test/testgen.cc (original)
+++ hadoop/avro/trunk/src/c++/test/testgen.cc Thu Nov 19 19:07:39 2009
@@ -29,6 +29,7 @@
#include "ValidatingWriter.hh"
#include "Reader.hh"
#include "ValidatingReader.hh"
+#include "Node.hh"
#include "ValidSchema.hh"
#include "Compiler.hh"
@@ -159,6 +160,24 @@
checkOk(myRecord, inRecord);
}
+void testNameIndex(const avro::ValidSchema &schema)
+{
+ const avro::NodePtr &node = schema.root();
+ size_t index = 0;
+ bool found = node->nameIndex("anothernested", index);
+ BOOST_CHECK_EQUAL(found, true);
+ BOOST_CHECK_EQUAL(index, 8U);
+
+ found = node->nameIndex("myenum", index);
+ BOOST_CHECK_EQUAL(found, true);
+ BOOST_CHECK_EQUAL(index, 4U);
+
+ const avro::NodePtr &enumNode = node->leafAt(index);
+ found = enumNode->nameIndex("one", index);
+ BOOST_CHECK_EQUAL(found, true);
+ BOOST_CHECK_EQUAL(index, 1U);
+}
+
void runTests(const testgen::RootRecord myRecord)
{
std::cout << "Serialize:\n";
@@ -172,6 +191,8 @@
serializeToScreenValid(schema, myRecord);
std::cout << "end Serialize validated\n";
+ testNameIndex(schema);
+
testParser(myRecord);
testParserValid(schema, myRecord);