Merge pull request #104 from ivajloip/fix-memory-leak-when-parsing-ma…

…ny-modules

Fix memory leak when parsing multiple modules.

Do this by creating an internal type dictionary for the `Modules` object, and then making the internal functions modify a type dictionary passed in as parameters, rather than directly modifying the global type dictionary.

pkg/yang/ast.go

@@ -65,10 +65,6 @@ type meta struct {
 	Module []*Module `yang:"module"`
 }
 
-func init() {
-	initTypes(reflect.TypeOf(&meta{}))
-}
-
 // aliases is a map of "aliased" names, that is, two types of statements
 // that parse (nearly) the same.
 var aliases = map[string]string{
@@ -78,24 +74,35 @@ var aliases = map[string]string{
 // BuildAST builds an abstract syntax tree based on the yang statement s.
 // Normally it should return a *Module.
 func BuildAST(s *Statement) (Node, error) {
-	v, err := build(s, nilValue)
+	return buildASTWithTypeDict(s, &typeDict)
+}
+
+// buildASTWithTypeDict creates an AST for the input statement, and returns its
+// root node. It also takes as input a type dictionary into which any
+// encountered typedefs within the statement are cached.
+func buildASTWithTypeDict(s *Statement, d *typeDictionary) (Node, error) {
+	initTypes(reflect.TypeOf(&meta{}), d)
+	v, err := build(s, nilValue, d)
 	if err != nil {
 		return nil, err
 	}
 	return v.Interface().(Node), nil
 }
 
-// build builds and returns an AST from the statement s, with parent p, or
-// returns an error.  The type of value returned depends on the keyword in s.
-func build(s *Statement, p reflect.Value) (v reflect.Value, err error) {
+// build builds and returns an AST from the statement s and with parent p. It
+// also takes as input a type dictionary d into which any encountered typedefs
+// within the statement are cached. The type of value returned depends on the
+// keyword in s. It returns an error if it cannot build the statement into its
+// corresponding Node type.
+func build(s *Statement, p reflect.Value, d *typeDictionary) (v reflect.Value, err error) {
 	defer func() {
 		// If we are returning a real Node then call addTypedefs
 		// if the node possibly contains typedefs.
 		if err != nil || v == nilValue {
 			return
 		}
 		if t, ok := v.Interface().(Typedefer); ok {
-			addTypedefs(t)
+			d.addTypedefs(t)
 		}
 	}()
 	kind := s.Keyword
@@ -197,7 +204,9 @@ func build(s *Statement, p reflect.Value) (v reflect.Value, err error) {
 }
 
 // initTypes builds up the functions necessary to parse a Statement into the
-// type at.  at must be a of type pointer to structure and that structure should
+// type at. It also builds up the functions to populate the input type
+// dictionary d with any encountered typedefs within the statement. at must be
+// a of type pointer to structure and that structure should
 // implement Node.  For each field of the structure with a yang tag (e.g.,
 // `yang:"command"`), a function is created and "command" is mapped to it.  The
 // complete map is then added to the typeMap map with at as the key.
@@ -258,7 +267,7 @@ func build(s *Statement, p reflect.Value) (v reflect.Value, err error) {
 //                   (This is to support merging Module and SubModule).
 //
 // If at contains substructures, initTypes recurses on the substructures.
-func initTypes(at reflect.Type) {
+func initTypes(at reflect.Type, d *typeDictionary) {
 	if typeMap[at] != nil {
 		return // we already defined this type
 	}
@@ -323,7 +332,7 @@ func initTypes(at reflect.Type) {
 			switch nameMap[name] {
 			case nil:
 				nameMap[name] = dt
-				initTypes(dt) // Make sure that structure type is included
+				initTypes(dt, d) // Make sure that structure type is included
 			case dt:
 			default:
 				panic("redeclared type " + name)
@@ -400,7 +409,7 @@ func initTypes(at reflect.Type) {
 				}
 
 				// Use build to build the value for this field.
-				sv, err := build(s, v)
+				sv, err := build(s, v, d)
 				if err != nil {
 					return err
 				}
@@ -423,7 +432,7 @@ func initTypes(at reflect.Type) {
 					if v.Type() != at {
 						panic(fmt.Sprintf("given type %s, need type %s", v.Type(), at))
 					}
-					sv, err := build(s, v)
+					sv, err := build(s, v, d)
 					if err != nil {
 						return err
 					}

pkg/yang/ast_test.go

@@ -207,7 +207,7 @@ func TestAST(t *testing.T) {
 	type meta struct {
 		MainNode []*MainNode `yang:"main_node"`
 	}
-	initTypes(reflect.TypeOf(&meta{}))
+	initTypes(reflect.TypeOf(&meta{}), &typeDict)
 
 	for _, tt := range []struct {
 		line int

pkg/yang/entry.go

@@ -535,6 +535,7 @@ func ToEntry(n Node) (e *Entry) {
 			Errors: []error{err},
 		}
 	}
+	ms := RootNode(n).modules
 	if e := entryCache[n]; e != nil {
 		return e
 	}
@@ -572,7 +573,7 @@ func ToEntry(n Node) (e *Entry) {
 	switch s := n.(type) {
 	case *Leaf:
 		e := newLeaf(n)
-		if errs := s.Type.resolve(); errs != nil {
+		if errs := s.Type.resolve(ms.typeDict); errs != nil {
 			e.Errors = errs
 		}
 		if s.Description != nil {
@@ -862,7 +863,7 @@ func ToEntry(n Node) (e *Entry) {
 			}
 
 			if n.Type != nil {
-				if errs := n.Type.resolve(); errs != nil {
+				if errs := n.Type.resolve(ms.typeDict); errs != nil {
 					e.addError(fmt.Errorf("deviation has unresolvable type, %v", errs))
 					continue
 				}
@@ -894,7 +895,7 @@ func ToEntry(n Node) (e *Entry) {
 
 					for _, sd := range d.Deviate {
 						if sd.Type != nil {
-							sd.Type.resolve()
+							sd.Type.resolve(ms.typeDict)
 						}
 					}
 				}

pkg/yang/modules.go

@@ -28,6 +28,7 @@ type Modules struct {
 	includes   map[*Module]bool   // Modules we have already done include on
 	byPrefix   map[string]*Module // Cache of prefix lookup
 	byNS       map[string]*Module // Cache of namespace lookup
+	typeDict   *typeDictionary    // Cache for type definitions.
 }
 
 // NewModules returns a newly created and initialized Modules.
@@ -38,6 +39,7 @@ func NewModules() *Modules {
 		includes:   map[*Module]bool{},
 		byPrefix:   map[string]*Module{},
 		byNS:       map[string]*Module{},
+		typeDict:   &typeDictionary{dict: map[Node]map[string]*Typedef{}},
 	}
 }
 
@@ -61,7 +63,7 @@ func (ms *Modules) Parse(data, name string) error {
 		return err
 	}
 	for _, s := range ss {
-		n, err := BuildAST(s)
+		n, err := buildASTWithTypeDict(s, ms.typeDict)
 		if err != nil {
 			return err
 		}
@@ -283,7 +285,7 @@ func (ms *Modules) process() []error {
 	// has not yet been built.
 	errs = append(errs, ms.resolveIdentities()...)
 	// Append any errors found trying to resolve typedefs
-	errs = append(errs, resolveTypedefs()...)
+	errs = append(errs, ms.typeDict.resolveTypedefs()...)
 
 	return errs
 }

pkg/yang/types.go

@@ -92,37 +92,37 @@ func (d *typeDictionary) typedefs() []*Typedef {
 // addTypedefs is called from BuildAST after each Typedefer is defined.  There
 // are no error conditions in this process as it is simply used to build up the
 // typedef dictionary.
-func addTypedefs(t Typedefer) {
+func (d *typeDictionary) addTypedefs(t Typedefer) {
 	for _, td := range t.Typedefs() {
-		typeDict.add(t, td.Name, td)
+		d.add(t, td.Name, td)
 	}
 }
 
 // resolveTypedefs is called after all of modules and submodules have been read,
 // as well as their imports and includes.  It resolves all typedefs found in all
 // modules and submodules read in.
-func resolveTypedefs() []error {
+func (d *typeDictionary) resolveTypedefs() []error {
 	var errs []error
 
 	// When resolve typedefs, we may need to look up other typedefs.
 	// We gather all typedefs into a slice so we don't deadlock on
 	// typeDict.
-	for _, td := range typeDict.typedefs() {
-		errs = append(errs, td.resolve()...)
+	for _, td := range d.typedefs() {
+		errs = append(errs, td.resolve(d)...)
 	}
 	return errs
 }
 
 // resolve creates a YangType for t, if not already done.  Resolving t
 // requires resolving the Type that t is based on.
-func (t *Typedef) resolve() []error {
+func (t *Typedef) resolve(d *typeDictionary) []error {
 	// If we have no parent we are a base type and
 	// are already resolved.
 	if t.Parent == nil || t.YangType != nil {
 		return nil
 	}
 
-	if errs := t.Type.resolve(); len(errs) != 0 {
+	if errs := t.Type.resolve(d); len(errs) != 0 {
 		return errs
 	}
 
@@ -158,7 +158,7 @@ func (t *Typedef) resolve() []error {
 
 // resolve resolves Type t, as well as the underlying typedef for t.  If t
 // cannot be resolved then one or more errors are returned.
-func (t *Type) resolve() (errs []error) {
+func (t *Type) resolve(d *typeDictionary) (errs []error) {
 	if t.YangType != nil {
 		return nil
 	}
@@ -182,13 +182,13 @@ check:
 		// If we have no prefix, or the prefix is what we call our own
 		// root, then we look in our ancestors for a typedef of name.
 		for n := Node(t); n != nil; n = n.ParentNode() {
-			if td = typeDict.find(n, name); td != nil {
+			if td = d.find(n, name); td != nil {
 				break check
 			}
 		}
 		// We need to check our sub-modules as well
 		for _, in := range root.Include {
-			if td = typeDict.find(in.Module, name); td != nil {
+			if td = d.find(in.Module, name); td != nil {
 				break check
 			}
 		}
@@ -208,12 +208,12 @@ check:
 		// what module it is part of and if it is defined at the top
 		// level of that module.
 		var err error
-		td, err = typeDict.findExternal(t, prefix, name)
+		td, err = d.findExternal(t, prefix, name)
 		if err != nil {
 			return []error{err}
 		}
 	}
-	if errs := td.resolve(); len(errs) > 0 {
+	if errs := td.resolve(d); len(errs) > 0 {
 		return errs
 	}
 
@@ -398,7 +398,7 @@ check:
 	// so we have to check equality the hard way.
 looking:
 	for _, ut := range t.Type {
-		errs = append(errs, ut.resolve()...)
+		errs = append(errs, ut.resolve(d)...)
 		if ut.YangType != nil {
 			for _, yt := range y.Type {
 				if ut.YangType.Equal(yt) {

pkg/yang/types_test.go

@@ -82,7 +82,7 @@ func TestTypeResolve(t *testing.T) {
 		// in Type.
 	} {
 		// We can initialize a value to ourself, so to it here.
-		errs := tt.in.resolve()
+		errs := tt.in.resolve(&typeDict)
 
 		// TODO(borman):  Do not hack out Root and Base.  These
 		// are hacked out for now because they can be self-referential,