BUG-6757: revert fix for BUG-4456

The fix has introduced a massive memory leak, which causes
all of temporary build objects to be retained in the final
SchemaContext.

Instead of the leak, add an explicit guard to detect
extensions (transitively) referencing themselves.

Change-Id: If90a4f9420866a6392ce97c71837915fa41ae0c8
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit e5226bf4ddd7a42aa13c392b2fc4d02d27ae2f74)

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java
index 062a3e90a7804fafbe3bd5155e8149cd1ec262d6..9a09f2b4abe1454ef16dc609524a5f57791cb2b3 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java
@@ -7,6 +7,8 @@
  */
 package org.opendaylight.yangtools.yang.parser.stmt.rfc6020;
 
+import java.util.HashSet;
+import java.util.Set;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.Rfc6020Mapping;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
@@ -20,6 +22,7 @@ import org.opendaylight.yangtools.yang.parser.spi.SubstatementValidator;
 import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractDeclaredStatement;
 import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractStatementSupport;
 import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext;
+import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
 import org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective.ExtensionEffectiveStatementImpl;
 
 public class ExtensionStatementImpl extends AbstractDeclaredStatement<QName> implements ExtensionStatement {
@@ -31,29 +34,48 @@ public class ExtensionStatementImpl extends AbstractDeclaredStatement<QName> imp
             .add(Rfc6020Mapping.STATUS, 0, 1)
             .build();
 
-    protected ExtensionStatementImpl(StmtContext<QName, ExtensionStatement,?> context) {
+    protected ExtensionStatementImpl(final StmtContext<QName, ExtensionStatement,?> context) {
         super(context);
     }
 
     public static class Definition extends AbstractStatementSupport<QName,ExtensionStatement,EffectiveStatement<QName,ExtensionStatement>> {
+        private static final ThreadLocal<Set<StmtContext<?, ?, ?>>> BUILDING = new ThreadLocal<>();
 
         public Definition() {
             super(Rfc6020Mapping.EXTENSION);
         }
 
         @Override
-        public QName parseArgumentValue(StmtContext<?,?,?> ctx, String value) {
+        public QName parseArgumentValue(final StmtContext<?,?,?> ctx, final String value) {
             return Utils.qNameFromArgument(ctx, value);
         }
 
         @Override
-        public ExtensionStatement createDeclared(StmtContext<QName, ExtensionStatement,?> ctx) {
+        public ExtensionStatement createDeclared(final StmtContext<QName, ExtensionStatement,?> ctx) {
             return new ExtensionStatementImpl(ctx);
         }
 
         @Override
-        public EffectiveStatement<QName,ExtensionStatement> createEffective(StmtContext<QName,ExtensionStatement,EffectiveStatement<QName,ExtensionStatement>> ctx) {
-           return new ExtensionEffectiveStatementImpl(ctx);
+        public EffectiveStatement<QName,ExtensionStatement> createEffective(
+                final StmtContext<QName,ExtensionStatement, EffectiveStatement<QName,ExtensionStatement>> ctx) {
+            Set<StmtContext<?, ?, ?>> building = BUILDING.get();
+            if (building == null) {
+                building = new HashSet<>();
+                BUILDING.set(building);
+            }
+
+            SourceException.throwIf(building.contains(ctx), ctx.getStatementSourceReference(),
+                "Extension %s references itself", ctx.getStatementArgument());
+
+            building.add(ctx);
+            try {
+                return new ExtensionEffectiveStatementImpl(ctx);
+            } finally {
+                building.remove(ctx);
+                if (building.isEmpty()) {
+                    BUILDING.remove();
+                }
+            }
         }
 
         @Override
@@ -62,7 +84,7 @@ public class ExtensionStatementImpl extends AbstractDeclaredStatement<QName> imp
         }
 
         @Override
-        public void onFullDefinitionDeclared(StmtContext.Mutable<QName, ExtensionStatement,
+        public void onFullDefinitionDeclared(final StmtContext.Mutable<QName, ExtensionStatement,
                 EffectiveStatement<QName, ExtensionStatement>> stmt) {
             super.onFullDefinitionDeclared(stmt);
             SUBSTATEMENT_VALIDATOR.validate(stmt);

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java
index 6ff829e21d35d96674c5b0c68a30b133775bf436..ce4eeff53bf1ced17cd67884bedc1f3ec9aa2db3 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java
@@ -19,24 +19,14 @@ public abstract class AbstractEffectiveDocumentedNode<A, D extends DeclaredState
     private final String reference;
     private final Status status;
 
-    protected AbstractEffectiveDocumentedNode(final StmtContext<A, D, ?> ctx) {
-        this(ctx, true);
-    }
-
     /**
      * Constructor.
      *
      * @param ctx
      *            context of statement.
-     * @param buildUnknownSubstatements
-     *            if it is false, the unknown substatements are omitted from
-     *            build of effective substatements till the call of either
-     *            effectiveSubstatements or getOmittedUnknownSubstatements
-     *            method of EffectiveStatementBase class. The main purpose of
-     *            this is to allow the build of recursive extension definitions.
      */
-    protected AbstractEffectiveDocumentedNode(final StmtContext<A, D, ?> ctx, boolean buildUnknownSubstatements) {
-        super(ctx, buildUnknownSubstatements);
+    protected AbstractEffectiveDocumentedNode(final StmtContext<A, D, ?> ctx) {
+        super(ctx);
 
         final DescriptionEffectiveStatementImpl descStmt = firstEffective(DescriptionEffectiveStatementImpl.class);
         if (descStmt != null) {

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java
index b4c8fe965c217f49410ba3e2357c414bfab4590b..db7aa9ebed2fb2458c4b312eedb047124035517d 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java
@@ -176,10 +176,7 @@ abstract class AbstractEffectiveModule<D extends DeclaredStatement<String>> exte
                 featuresInit.add((FeatureDefinition) effectiveStatement);
             }
             if (effectiveStatement instanceof ExtensionEffectiveStatementImpl) {
-                ExtensionEffectiveStatementImpl extensionDefinition = (ExtensionEffectiveStatementImpl) effectiveStatement;
-                extensionDefinition.initUnknownSchemaNodes();
-                extensionNodesInit
-                        .add(extensionDefinition);
+                extensionNodesInit.add((ExtensionEffectiveStatementImpl) effectiveStatement);
             }
             if (effectiveStatement instanceof DataSchemaNode) {
                 DataSchemaNode dataSchemaNode = (DataSchemaNode) effectiveStatement;

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java
index e460303861fbef008e0e9314967c45e6c5d434b3..a1fdda30be028f2e99a1710f916d804ae6c4fa31 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java
@@ -21,24 +21,14 @@ public abstract class DeclaredEffectiveStatementBase<A, D extends DeclaredStatem
     private final A argument;
     private final D declaredInstance;
 
-    protected DeclaredEffectiveStatementBase(final StmtContext<A, D, ?> ctx) {
-        this(ctx, true);
-    }
-
     /**
      * Constructor.
      *
      * @param ctx
      *            context of statement.
-     * @param buildUnknownSubstatements
-     *            if it is false, the unknown substatements are omitted from
-     *            build of effective substatements till the call of either
-     *            effectiveSubstatements or getOmittedUnknownSubstatements
-     *            method of EffectiveStatementBase class. The main purpose of
-     *            this is to allow the build of recursive extension definitions.
      */
-    protected DeclaredEffectiveStatementBase(StmtContext<A, D, ?> ctx, final boolean buildUnknownSubstatements) {
-        super(ctx, buildUnknownSubstatements);
+    protected DeclaredEffectiveStatementBase(StmtContext<A, D, ?> ctx) {
+        super(ctx);
 
         this.argument = ctx.getStatementArgument();
         this.statementSource = ctx.getStatementSource();

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
index 7f55295465c586afec4228f49cc0ee55cf66480e..81374085f66da0ad9974985f5a67af9c506681c5 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java
@@ -8,7 +8,6 @@
 package org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective;
 
 import com.google.common.base.Optional;
-import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
@@ -27,32 +26,15 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils;
 import org.opendaylight.yangtools.yang.parser.stmt.reactor.StatementContextBase;
 
 public abstract class EffectiveStatementBase<A, D extends DeclaredStatement<A>> implements EffectiveStatement<A, D> {
-
-    private static final Predicate<StmtContext<?, ?, ?>> IS_UNKNOWN_STATEMENT_CONTEXT =
-            StmtContextUtils::isUnknownStatement;
-    private static final Predicate<StmtContext<?, ?, ?>> IS_NOT_UNKNOWN_STATEMENT_CONTEXT =
-            Predicates.not(IS_UNKNOWN_STATEMENT_CONTEXT);
-
     private final List<? extends EffectiveStatement<?, ?>> substatements;
-    private final List<StatementContextBase<?, ?, ?>> unknownSubstatementsToBuild;
-
-    protected EffectiveStatementBase(final StmtContext<A, D, ?> ctx) {
-        this(ctx, true);
-    }
 
     /**
      * Constructor.
      *
      * @param ctx
      *            context of statement.
-     * @param buildUnknownSubstatements
-     *            if it is false, the unknown substatements are omitted from
-     *            build of effective substatements till the call of either
-     *            effectiveSubstatements or getOmittedUnknownSubstatements
-     *            method. The main purpose of this is to allow the build of
-     *            recursive extension definitions.
      */
-    protected EffectiveStatementBase(final StmtContext<A, D, ?> ctx, final boolean buildUnknownSubstatements) {
+    protected EffectiveStatementBase(final StmtContext<A, D, ?> ctx) {
 
         final Collection<StatementContextBase<?, ?, ?>> effectiveSubstatements = ctx.effectiveSubstatements();
         final Collection<StatementContextBase<?, ?, ?>> substatementsInit = new ArrayList<>();
@@ -71,21 +53,8 @@ public abstract class EffectiveStatementBase<A, D extends DeclaredStatement<A>>
         }
         substatementsInit.addAll(effectiveSubstatements);
 
-        Collection<StatementContextBase<?, ?, ?>> substatementsToBuild = Collections2.filter(substatementsInit,
-            StmtContext::isSupportedToBuildEffective);
-        if (!buildUnknownSubstatements) {
-            this.unknownSubstatementsToBuild = ImmutableList.copyOf(Collections2.filter(substatementsToBuild,
-                    IS_UNKNOWN_STATEMENT_CONTEXT));
-            substatementsToBuild = Collections2.filter(substatementsToBuild, IS_NOT_UNKNOWN_STATEMENT_CONTEXT);
-        } else {
-            this.unknownSubstatementsToBuild = ImmutableList.of();
-        }
-
-        this.substatements = ImmutableList.copyOf(Collections2.transform(substatementsToBuild, StatementContextBase::buildEffective));
-    }
-
-    Collection<EffectiveStatement<?, ?>> getOmittedUnknownSubstatements() {
-        return Collections2.transform(unknownSubstatementsToBuild, StatementContextBase::buildEffective);
+        this.substatements = ImmutableList.copyOf(Collections2.transform(Collections2.filter(substatementsInit,
+            StmtContext::isSupportedToBuildEffective), StatementContextBase::buildEffective));
     }
 
     @Override
@@ -100,11 +69,7 @@ public abstract class EffectiveStatementBase<A, D extends DeclaredStatement<A>>
 
     @Override
     public final Collection<? extends EffectiveStatement<?, ?>> effectiveSubstatements() {
-        if (unknownSubstatementsToBuild.isEmpty()) {
-            return substatements;
-        } else {
-            return ImmutableList.copyOf(Iterables.concat(substatements, getOmittedUnknownSubstatements()));
-        }
+        return substatements;
     }
 
     protected final <S extends EffectiveStatement<?, ?>> S firstEffective(final Class<S> type) {

diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java
index da4a2a92ec937e6e6c2fbec70fa2ef3b46a6f283..fd9623b0bc3f0c59a7f06a387e2f9e7ae3922f42 100644 (file)
--- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java
+++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java
@@ -9,7 +9,6 @@ package org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective;
 
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import org.opendaylight.yangtools.yang.common.QName;
@@ -26,15 +25,23 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented
     private final String argument;
     private final SchemaPath schemaPath;
 
-    private List<UnknownSchemaNode> unknownNodes;
+    private final List<UnknownSchemaNode> unknownNodes;
     private final boolean yin;
 
     public ExtensionEffectiveStatementImpl(
             final StmtContext<QName, ExtensionStatement, EffectiveStatement<QName, ExtensionStatement>> ctx) {
-        super(ctx, false);
+        super(ctx);
         this.qname = ctx.getStatementArgument();
         this.schemaPath = ctx.getSchemaPath().get();
 
+        final List<UnknownSchemaNode> unknownNodesInit = new ArrayList<>();
+        for (EffectiveStatement<?, ?> unknownNode : effectiveSubstatements()) {
+            if (unknownNode instanceof UnknownSchemaNode) {
+                unknownNodesInit.add((UnknownSchemaNode) unknownNode);
+            }
+        }
+        this.unknownNodes = ImmutableList.copyOf(unknownNodesInit);
+
         // initFields
         ArgumentEffectiveStatementImpl argumentSubstatement = firstEffective(ArgumentEffectiveStatementImpl.class);
         if (argumentSubstatement != null) {
@@ -53,21 +60,6 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented
         }
     }
 
-    void initUnknownSchemaNodes() {
-        if (unknownNodes != null) {
-            return;
-        }
-
-        Collection<EffectiveStatement<?, ?>> buildedUnknownNodes = getOmittedUnknownSubstatements();
-        List<UnknownSchemaNode> unknownNodesInit = new ArrayList<>();
-        for (EffectiveStatement<?, ?> unknownNode : buildedUnknownNodes) {
-            if (unknownNode instanceof UnknownSchemaNode) {
-                unknownNodesInit.add((UnknownSchemaNode) unknownNode);
-            }
-        }
-        this.unknownNodes = ImmutableList.copyOf(unknownNodesInit);
-    }
-
     @Override
     public QName getQName() {
         return qname;
@@ -80,9 +72,6 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented
 
     @Override
     public List<UnknownSchemaNode> getUnknownSchemaNodes() {
-        if (unknownNodes == null) {
-            initUnknownSchemaNodes();
-        }
         return unknownNodes;
     }
 

diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java
index 01cea68b48ceec021899fb95e044f3a92aca4135..3fa3f208cfca877944339f2ceec2d8f6e93e7565 100644 (file)
--- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java
+++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java
@@ -22,10 +22,11 @@ import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode;
 import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException;
+import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException;
 import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
 
 public class Bug4456Test {
-    @Test
+    @Test(expected=SomeModifiersUnresolvedException.class)
     public void test() throws IOException, URISyntaxException, SourceException, ReactorException {
         SchemaContext schema = StmtTestUtils.parseYangSources("/bugs/bug4456");
         assertNotNull(schema);